diff options
132 files changed, 1775 insertions, 923 deletions
diff --git a/cmds/dumpstate/Android.bp b/cmds/dumpstate/Android.bp index b22cc2a508..372008e63b 100644 --- a/cmds/dumpstate/Android.bp +++ b/cmds/dumpstate/Android.bp @@ -118,6 +118,12 @@ cc_defaults { ], } +prebuilt_etc { + name: "default_screenshot", + src: "res/default_screenshot.png", + filename_from_src: true, +} + cc_binary { name: "dumpstate", defaults: ["dumpstate_defaults"], @@ -130,6 +136,7 @@ cc_binary { required: [ "atrace", "bugreport_procdump", + "default_screenshot", "dmabuf_dump", "ip", "iptables", diff --git a/cmds/dumpstate/dumpstate.cpp b/cmds/dumpstate/dumpstate.cpp index d427ecf280..d24edc4d82 100644 --- a/cmds/dumpstate/dumpstate.cpp +++ b/cmds/dumpstate/dumpstate.cpp @@ -206,6 +206,9 @@ static const std::string ANR_TRACE_FILE_PREFIX = "trace_"; static const std::string SHUTDOWN_CHECKPOINTS_DIR = "/data/system/shutdown-checkpoints/"; static const std::string SHUTDOWN_CHECKPOINTS_FILE_PREFIX = "checkpoints-"; +// File path to default screenshot image, that used when failed to capture the real screenshot. +static const std::string DEFAULT_SCREENSHOT_PATH = "/system/etc/default_screenshot.png"; + // TODO: temporary variables and functions used during C++ refactoring #define RETURN_IF_USER_DENIED_CONSENT() \ @@ -765,10 +768,14 @@ android::binder::Status Dumpstate::ConsentCallback::onReportApproved() { bool copy_succeeded = android::os::CopyFileToFd(ds.screenshot_path_, ds.options_->screenshot_fd.get()); - ds.options_->is_screenshot_copied = copy_succeeded; if (copy_succeeded) { android::os::UnlinkAndLogOnError(ds.screenshot_path_); + } else { + MYLOGE("Failed to copy screenshot to a permanent file.\n"); + copy_succeeded = android::os::CopyFileToFd(DEFAULT_SCREENSHOT_PATH, + ds.options_->screenshot_fd.get()); } + ds.options_->is_screenshot_copied = copy_succeeded; return android::binder::Status::ok(); } @@ -3442,7 +3449,9 @@ Dumpstate::RunStatus Dumpstate::RunInternal(int32_t calling_uid, // Do an early return if there were errors. We make an exception for consent // timing out because it's possible the user got distracted. In this case the // bugreport is not shared but made available for manual retrieval. - MYLOGI("User denied consent. Returning\n"); + MYLOGI("Bug report generation failed, this could have been due to" + " several reasons such as BR copy failed, user consent was" + " not grated etc. Returning\n"); return status; } if (status == Dumpstate::RunStatus::USER_CONSENT_TIMED_OUT) { @@ -3729,12 +3738,16 @@ Dumpstate::RunStatus Dumpstate::CopyBugreportIfUserConsented(int32_t calling_uid if (options_->do_screenshot && options_->screenshot_fd.get() != -1 && !options_->is_screenshot_copied) { - copy_succeeded = android::os::CopyFileToFd(screenshot_path_, + bool is_screenshot_copied = android::os::CopyFileToFd(screenshot_path_, options_->screenshot_fd.get()); - options_->is_screenshot_copied = copy_succeeded; - if (copy_succeeded) { + if (is_screenshot_copied) { android::os::UnlinkAndLogOnError(screenshot_path_); + } else { + MYLOGE("Failed to copy screenshot to a permanent file.\n"); + is_screenshot_copied = android::os::CopyFileToFd(DEFAULT_SCREENSHOT_PATH, + options_->screenshot_fd.get()); } + options_->is_screenshot_copied = is_screenshot_copied; } } return copy_succeeded ? Dumpstate::RunStatus::OK : Dumpstate::RunStatus::ERROR; @@ -3825,7 +3838,7 @@ DurationReporter::DurationReporter(const std::string& title, bool logcat_only, b DurationReporter::~DurationReporter() { if (!title_.empty()) { float elapsed = (float)(Nanotime() - started_) / NANOS_PER_SEC; - if (elapsed >= .5f || verbose_) { + if (elapsed >= 1.0f || verbose_) { MYLOGD("Duration of '%s': %.2fs\n", title_.c_str(), elapsed); } if (!logcat_only_) { diff --git a/cmds/dumpstate/res/default_screenshot.png b/cmds/dumpstate/res/default_screenshot.png Binary files differnew file mode 100644 index 0000000000..10f36aa52b --- /dev/null +++ b/cmds/dumpstate/res/default_screenshot.png diff --git a/data/etc/android.software.opengles.deqp.level-2023-03-01.xml b/data/etc/android.software.opengles.deqp.level-2023-03-01.xml index d0b594c73d..7aef9ec6c4 100644 --- a/data/etc/android.software.opengles.deqp.level-2023-03-01.xml +++ b/data/etc/android.software.opengles.deqp.level-2023-03-01.xml @@ -1,5 +1,5 @@ <?xml version="1.0" encoding="utf-8"?> -<!-- Copyright 2021 The Android Open Source Project +<!-- Copyright 2023 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. diff --git a/data/etc/android.software.opengles.deqp.level-2024-03-01.xml b/data/etc/android.software.opengles.deqp.level-2024-03-01.xml index 4eeed2af4f..3d8a17819c 100644 --- a/data/etc/android.software.opengles.deqp.level-2024-03-01.xml +++ b/data/etc/android.software.opengles.deqp.level-2024-03-01.xml @@ -1,5 +1,5 @@ <?xml version="1.0" encoding="utf-8"?> -<!-- Copyright 2021 The Android Open Source Project +<!-- Copyright 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. @@ -15,7 +15,7 @@ --> <!-- This is the standard feature indicating that the device passes OpenGL ES - dEQP tests associated with date 2023-03-01 (0x07E70301). --> + dEQP tests associated with date 2024-03-01 (0x7e80301). --> <permissions> <feature name="android.software.opengles.deqp.level" version="132645633" /> </permissions> diff --git a/data/etc/android.software.opengles.deqp.level-2025-03-01.xml b/data/etc/android.software.opengles.deqp.level-2025-03-01.xml new file mode 100644 index 0000000000..e005356555 --- /dev/null +++ b/data/etc/android.software.opengles.deqp.level-2025-03-01.xml @@ -0,0 +1,21 @@ +<?xml version="1.0" encoding="utf-8"?> +<!-- Copyright 2025 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. +--> + +<!-- This is the standard feature indicating that the device passes OpenGL ES + dEQP tests associated with date 2025-03-01 (0x7e90301). --> +<permissions> + <feature name="android.software.opengles.deqp.level" version="132711169" /> +</permissions> diff --git a/data/etc/android.software.opengles.deqp.level-latest.xml b/data/etc/android.software.opengles.deqp.level-latest.xml index 62bb10161a..1ad35bce07 100644 --- a/data/etc/android.software.opengles.deqp.level-latest.xml +++ b/data/etc/android.software.opengles.deqp.level-latest.xml @@ -1,5 +1,5 @@ <?xml version="1.0" encoding="utf-8"?> -<!-- Copyright 2023 The Android Open Source Project +<!-- Copyright 2025 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. @@ -17,5 +17,5 @@ <!-- This is the standard feature indicating that the device passes OpenGL ES dEQP tests associated with the most recent level for this Android version. --> <permissions> - <feature name="android.software.opengles.deqp.level" version="132645633" /> + <feature name="android.software.opengles.deqp.level" version="132711169" /> </permissions> diff --git a/data/etc/android.software.vulkan.deqp.level-2024-03-01.xml b/data/etc/android.software.vulkan.deqp.level-2024-03-01.xml index 8b2b4c8199..185edbf4bd 100644 --- a/data/etc/android.software.vulkan.deqp.level-2024-03-01.xml +++ b/data/etc/android.software.vulkan.deqp.level-2024-03-01.xml @@ -1,5 +1,5 @@ <?xml version="1.0" encoding="utf-8"?> -<!-- Copyright 2021 The Android Open Source Project +<!-- Copyright 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. @@ -15,7 +15,7 @@ --> <!-- This is the standard feature indicating that the device passes Vulkan dEQP - tests associated with date 2023-03-01 (0x07E70301). --> + tests associated with date 2024-03-01 (0x7e80301). --> <permissions> <feature name="android.software.vulkan.deqp.level" version="132645633" /> </permissions> diff --git a/data/etc/android.software.vulkan.deqp.level-2025-03-01.xml b/data/etc/android.software.vulkan.deqp.level-2025-03-01.xml new file mode 100644 index 0000000000..b424667b3a --- /dev/null +++ b/data/etc/android.software.vulkan.deqp.level-2025-03-01.xml @@ -0,0 +1,21 @@ +<?xml version="1.0" encoding="utf-8"?> +<!-- Copyright 2025 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. +--> + +<!-- This is the standard feature indicating that the device passes Vulkan dEQP + tests associated with date 2025-03-01 (0x7e90301). --> +<permissions> + <feature name="android.software.vulkan.deqp.level" version="132711169" /> +</permissions> diff --git a/data/etc/android.software.vulkan.deqp.level-latest.xml b/data/etc/android.software.vulkan.deqp.level-latest.xml index 0fc12b3b5f..4128a95ffd 100644 --- a/data/etc/android.software.vulkan.deqp.level-latest.xml +++ b/data/etc/android.software.vulkan.deqp.level-latest.xml @@ -1,5 +1,5 @@ <?xml version="1.0" encoding="utf-8"?> -<!-- Copyright 2023 The Android Open Source Project +<!-- Copyright 2025 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. @@ -17,5 +17,5 @@ <!-- This is the standard feature indicating that the device passes Vulkan dEQP tests associated with the most recent level for this Android version. --> <permissions> - <feature name="android.software.vulkan.deqp.level" version="132645633" /> + <feature name="android.software.vulkan.deqp.level" version="132711169" /> </permissions> diff --git a/include/android/surface_control.h b/include/android/surface_control.h index bf9acb37da..8d61e772cc 100644 --- a/include/android/surface_control.h +++ b/include/android/surface_control.h @@ -156,8 +156,6 @@ typedef struct ASurfaceTransactionStats ASurfaceTransactionStats; * * THREADING * The transaction completed callback can be invoked on any thread. - * - * Available since API level 29. */ typedef void (*ASurfaceTransaction_OnComplete)(void* _Null_unspecified context, ASurfaceTransactionStats* _Nonnull stats); @@ -184,8 +182,6 @@ typedef void (*ASurfaceTransaction_OnComplete)(void* _Null_unspecified context, * * THREADING * The transaction committed callback can be invoked on any thread. - * - * Available since API level 31. */ typedef void (*ASurfaceTransaction_OnCommit)(void* _Null_unspecified context, ASurfaceTransactionStats* _Nonnull stats); @@ -213,8 +209,6 @@ typedef void (*ASurfaceTransaction_OnCommit)(void* _Null_unspecified context, * * THREADING * The callback can be invoked on any thread. - * - * Available since API level 36. */ typedef void (*ASurfaceTransaction_OnBufferRelease)(void* _Null_unspecified context, int release_fence_fd); diff --git a/include/ftl/non_null.h b/include/ftl/non_null.h index 4a5d8bffd0..e68428810b 100644 --- a/include/ftl/non_null.h +++ b/include/ftl/non_null.h @@ -68,6 +68,15 @@ class NonNull final { constexpr NonNull(const NonNull&) = default; constexpr NonNull& operator=(const NonNull&) = default; + template <typename U, typename = std::enable_if_t<std::is_convertible_v<U, Pointer>>> + constexpr NonNull(const NonNull<U>& other) : pointer_(other.get()) {} + + template <typename U, typename = std::enable_if_t<std::is_convertible_v<U, Pointer>>> + constexpr NonNull& operator=(const NonNull<U>& other) { + pointer_ = other.get(); + return *this; + } + [[nodiscard]] constexpr const Pointer& get() const { return pointer_; } [[nodiscard]] constexpr explicit operator const Pointer&() const { return get(); } diff --git a/libs/binder/Parcel.cpp b/libs/binder/Parcel.cpp index 4b7af45739..842ea77459 100644 --- a/libs/binder/Parcel.cpp +++ b/libs/binder/Parcel.cpp @@ -180,7 +180,7 @@ static void acquire_object(const sp<ProcessState>& proc, const flat_binder_objec } } - ALOGD("Invalid object type 0x%08x", obj.hdr.type); + ALOGE("Invalid object type 0x%08x to acquire", obj.hdr.type); } static void release_object(const sp<ProcessState>& proc, const flat_binder_object& obj, @@ -210,7 +210,7 @@ static void release_object(const sp<ProcessState>& proc, const flat_binder_objec } } - ALOGE("Invalid object type 0x%08x", obj.hdr.type); + ALOGE("Invalid object type 0x%08x to release", obj.hdr.type); } #endif // BINDER_WITH_KERNEL_IPC @@ -683,7 +683,7 @@ status_t Parcel::appendFrom(const Parcel* parcel, size_t offset, size_t len) { return err; } -int Parcel::compareData(const Parcel& other) { +int Parcel::compareData(const Parcel& other) const { size_t size = dataSize(); if (size != other.dataSize()) { return size < other.dataSize() ? -1 : 1; @@ -1150,31 +1150,6 @@ status_t Parcel::finishWrite(size_t len) return NO_ERROR; } -status_t Parcel::writeUnpadded(const void* data, size_t len) -{ - if (len > INT32_MAX) { - // don't accept size_t values which may have come from an - // inadvertent conversion from a negative int. - return BAD_VALUE; - } - - size_t end = mDataPos + len; - if (end < mDataPos) { - // integer overflow - return BAD_VALUE; - } - - if (end <= mDataCapacity) { -restart_write: - memcpy(mData+mDataPos, data, len); - return finishWrite(len); - } - - status_t err = growData(len); - if (err == NO_ERROR) goto restart_write; - return err; -} - status_t Parcel::write(const void* data, size_t len) { if (len > INT32_MAX) { @@ -1874,7 +1849,10 @@ status_t Parcel::validateReadData(size_t upperBound) const if (mDataPos < kernelFields->mObjects[nextObject] + sizeof(flat_binder_object)) { // Requested info overlaps with an object if (!mServiceFuzzing) { - ALOGE("Attempt to read from protected data in Parcel %p", this); + ALOGE("Attempt to read or write from protected data in Parcel %p. pos: " + "%zu, nextObject: %zu, object offset: %llu, object size: %zu", + this, mDataPos, nextObject, kernelFields->mObjects[nextObject], + sizeof(flat_binder_object)); } return PERMISSION_DENIED; } @@ -2948,6 +2926,14 @@ status_t Parcel::growData(size_t len) return BAD_VALUE; } + if (mDataPos > mDataSize) { + // b/370831157 - this case used to abort. We also don't expect mDataPos < mDataSize, but + // this would only waste a bit of memory, so it's okay. + ALOGE("growData only expected at the end of a Parcel. pos: %zu, size: %zu, capacity: %zu", + mDataPos, len, mDataCapacity); + return BAD_VALUE; + } + if (len > SIZE_MAX - mDataSize) return NO_MEMORY; // overflow if (mDataSize + len > SIZE_MAX / 3) return NO_MEMORY; // overflow size_t newSize = ((mDataSize+len)*3)/2; diff --git a/libs/binder/Status.cpp b/libs/binder/Status.cpp index dba65878fb..9a98097af8 100644 --- a/libs/binder/Status.cpp +++ b/libs/binder/Status.cpp @@ -99,27 +99,28 @@ status_t Status::readFromParcel(const Parcel& parcel) { return status; } - // Skip over fat response headers. Not used (or propagated) in native code. - if (mException == EX_HAS_REPLY_HEADER) { - // Note that the header size includes the 4 byte size field. - const size_t header_start = parcel.dataPosition(); - // Get available size before reading more - const size_t header_avail = parcel.dataAvail(); - - int32_t header_size; - status = parcel.readInt32(&header_size); + if (mException == EX_HAS_NOTED_APPOPS_REPLY_HEADER) { + status = skipUnusedHeader(parcel); + if (status != OK) { + setFromStatusT(status); + return status; + } + // Read next exception code. + status = parcel.readInt32(&mException); if (status != OK) { setFromStatusT(status); return status; } + } - if (header_size < 0 || static_cast<size_t>(header_size) > header_avail) { - android_errorWriteLog(0x534e4554, "132650049"); - setFromStatusT(UNKNOWN_ERROR); - return UNKNOWN_ERROR; + // Skip over fat response headers. Not used (or propagated) in native code. + if (mException == EX_HAS_REPLY_HEADER) { + status = skipUnusedHeader(parcel); + if (status != OK) { + setFromStatusT(status); + return status; } - parcel.setDataPosition(header_start + header_size); // And fat response headers are currently only used when there are no // exceptions, so act like there was no error. mException = EX_NONE; @@ -257,5 +258,28 @@ String8 Status::toString8() const { return ret; } +status_t Status::skipUnusedHeader(const Parcel& parcel) { + // Note that the header size includes the 4 byte size field. + const size_t header_start = parcel.dataPosition(); + // Get available size before reading more + const size_t header_avail = parcel.dataAvail(); + + int32_t header_size; + status_t status = parcel.readInt32(&header_size); + ALOGD("Skip unused header. exception code: %d, start: %zu, size: %d.", + mException, header_start, header_size); + if (status != OK) { + return status; + } + + if (header_size < 0 || static_cast<size_t>(header_size) > header_avail) { + android_errorWriteLog(0x534e4554, "132650049"); + return UNKNOWN_ERROR; + } + + parcel.setDataPosition(header_start + header_size); + return OK; +} + } // namespace binder } // namespace android diff --git a/libs/binder/include/binder/Parcel.h b/libs/binder/include/binder/Parcel.h index 5cc0830c88..ed4e211481 100644 --- a/libs/binder/include/binder/Parcel.h +++ b/libs/binder/include/binder/Parcel.h @@ -92,7 +92,7 @@ public: LIBBINDER_EXPORTED status_t appendFrom(const Parcel* parcel, size_t start, size_t len); - LIBBINDER_EXPORTED int compareData(const Parcel& other); + LIBBINDER_EXPORTED int compareData(const Parcel& other) const; LIBBINDER_EXPORTED status_t compareDataInRange(size_t thisOffset, const Parcel& other, size_t otherOffset, size_t length, int* result) const; @@ -172,7 +172,6 @@ public: LIBBINDER_EXPORTED status_t write(const void* data, size_t len); LIBBINDER_EXPORTED void* writeInplace(size_t len); - LIBBINDER_EXPORTED status_t writeUnpadded(const void* data, size_t len); LIBBINDER_EXPORTED status_t writeInt32(int32_t val); LIBBINDER_EXPORTED status_t writeUint32(uint32_t val); LIBBINDER_EXPORTED status_t writeInt64(int64_t val); @@ -637,9 +636,6 @@ public: LIBBINDER_EXPORTED const flat_binder_object* readObject(bool nullMetaData) const; - // Explicitly close all file descriptors in the parcel. - LIBBINDER_EXPORTED void closeFileDescriptors(); - // Debugging: get metrics on current allocations. LIBBINDER_EXPORTED static size_t getGlobalAllocSize(); LIBBINDER_EXPORTED static size_t getGlobalAllocCount(); @@ -652,6 +648,9 @@ public: LIBBINDER_EXPORTED void print(std::ostream& to, uint32_t flags = 0) const; private: + // Explicitly close all file descriptors in the parcel. + void closeFileDescriptors(); + // `objects` and `objectsSize` always 0 for RPC Parcels. typedef void (*release_func)(const uint8_t* data, size_t dataSize, const binder_size_t* objects, size_t objectsSize); diff --git a/libs/binder/include/binder/Status.h b/libs/binder/include/binder/Status.h index 49ccf7c36c..d69f662891 100644 --- a/libs/binder/include/binder/Status.h +++ b/libs/binder/include/binder/Status.h @@ -67,6 +67,9 @@ public: EX_SERVICE_SPECIFIC = -8, EX_PARCELABLE = -9, + // See android/os/Parcel.java. We need to handle this in native code. + EX_HAS_NOTED_APPOPS_REPLY_HEADER = -127, + // This is special and Java specific; see Parcel.java. EX_HAS_REPLY_HEADER = -128, // This is special, and indicates to C++ binder proxies that the @@ -150,6 +153,8 @@ private: Status(int32_t exceptionCode, int32_t errorCode); Status(int32_t exceptionCode, int32_t errorCode, const String8& message); + status_t skipUnusedHeader(const Parcel& parcel); + // If |mException| == EX_TRANSACTION_FAILED, generated code will return // |mErrorCode| as the result of the transaction rather than write an // exception to the reply parcel. diff --git a/libs/binder/ndk/binder_rpc.cpp b/libs/binder/ndk/binder_rpc.cpp index 2cc5f8117e..3c32a39e98 100644 --- a/libs/binder/ndk/binder_rpc.cpp +++ b/libs/binder/ndk/binder_rpc.cpp @@ -94,7 +94,7 @@ struct OnDeleteProviderHolder { } void* mData; ABinderRpc_AccessorProviderUserData_deleteCallback mOnDelete; - // needs to be copyable for std::function, but we will never copy it + // needs to be copy-able for std::function, but we will never copy it OnDeleteProviderHolder(const OnDeleteProviderHolder&) { LOG_ALWAYS_FATAL("This object can't be copied!"); } @@ -113,7 +113,7 @@ ABinderRpc_AccessorProvider* ABinderRpc_registerAccessorProvider( } if (data && onDelete == nullptr) { ALOGE("If a non-null data ptr is passed to ABinderRpc_registerAccessorProvider, then a " - "ABinderRpc_AccessorProviderUserData_deleteCallback must alse be passed to delete " + "ABinderRpc_AccessorProviderUserData_deleteCallback must also be passed to delete " "the data object once the ABinderRpc_AccessorProvider is removed."); return nullptr; } @@ -179,7 +179,7 @@ struct OnDeleteConnectionInfoHolder { } void* mData; ABinderRpc_ConnectionInfoProviderUserData_delete mOnDelete; - // needs to be copyable for std::function, but we will never copy it + // needs to be copy-able for std::function, but we will never copy it OnDeleteConnectionInfoHolder(const OnDeleteConnectionInfoHolder&) { LOG_ALWAYS_FATAL("This object can't be copied!"); } @@ -197,7 +197,7 @@ ABinderRpc_Accessor* ABinderRpc_Accessor_new( } if (data && onDelete == nullptr) { ALOGE("If a non-null data ptr is passed to ABinderRpc_Accessor_new, then a " - "ABinderRpc_ConnectionInfoProviderUserData_delete callback must alse be passed to " + "ABinderRpc_ConnectionInfoProviderUserData_delete callback must also be passed to " "delete " "the data object once the ABinderRpc_Accessor is deleted."); return nullptr; @@ -304,7 +304,7 @@ ABinderRpc_Accessor* ABinderRpc_Accessor_fromBinder(const char* instance, AIBind ABinderRpc_ConnectionInfo* ABinderRpc_ConnectionInfo_new(const sockaddr* addr, socklen_t len) { if (addr == nullptr || len < 0 || static_cast<size_t>(len) < sizeof(sa_family_t)) { - ALOGE("Invalid arguments in Arpc_Connection_new"); + ALOGE("Invalid arguments in ABinderRpc_Connection_new"); return nullptr; } // socklen_t was int32_t on 32-bit and uint32_t on 64 bit. @@ -317,8 +317,9 @@ ABinderRpc_ConnectionInfo* ABinderRpc_ConnectionInfo_new(const sockaddr* addr, s return nullptr; } sockaddr_vm vm = *reinterpret_cast<const sockaddr_vm*>(addr); - LOG_ACCESSOR_DEBUG("Arpc_ConnectionInfo_new found AF_VSOCK. family %d, port %d, cid %d", - vm.svm_family, vm.svm_port, vm.svm_cid); + LOG_ACCESSOR_DEBUG( + "ABinderRpc_ConnectionInfo_new found AF_VSOCK. family %d, port %d, cid %d", + vm.svm_family, vm.svm_port, vm.svm_cid); return new ABinderRpc_ConnectionInfo(vm); } else if (addr->sa_family == AF_UNIX) { if (len != sizeof(sockaddr_un)) { @@ -327,7 +328,7 @@ ABinderRpc_ConnectionInfo* ABinderRpc_ConnectionInfo_new(const sockaddr* addr, s return nullptr; } sockaddr_un un = *reinterpret_cast<const sockaddr_un*>(addr); - LOG_ACCESSOR_DEBUG("Arpc_ConnectionInfo_new found AF_UNIX. family %d, path %s", + LOG_ACCESSOR_DEBUG("ABinderRpc_ConnectionInfo_new found AF_UNIX. family %d, path %s", un.sun_family, un.sun_path); return new ABinderRpc_ConnectionInfo(un); } else if (addr->sa_family == AF_INET) { @@ -337,12 +338,14 @@ ABinderRpc_ConnectionInfo* ABinderRpc_ConnectionInfo_new(const sockaddr* addr, s return nullptr; } sockaddr_in in = *reinterpret_cast<const sockaddr_in*>(addr); - LOG_ACCESSOR_DEBUG("Arpc_ConnectionInfo_new found AF_INET. family %d, address %s, port %d", - in.sin_family, inet_ntoa(in.sin_addr), ntohs(in.sin_port)); + LOG_ACCESSOR_DEBUG( + "ABinderRpc_ConnectionInfo_new found AF_INET. family %d, address %s, port %d", + in.sin_family, inet_ntoa(in.sin_addr), ntohs(in.sin_port)); return new ABinderRpc_ConnectionInfo(in); } - ALOGE("ARpc APIs only support AF_VSOCK right now but the supplied sockadder::sa_family is: %hu", + ALOGE("ABinderRpc APIs only support AF_VSOCK right now but the supplied sockaddr::sa_family " + "is: %hu", addr->sa_family); return nullptr; } diff --git a/libs/binder/ndk/include_cpp/android/binder_interface_utils.h b/libs/binder/ndk/include_cpp/android/binder_interface_utils.h index 8296356d6b..9d68399145 100644 --- a/libs/binder/ndk/include_cpp/android/binder_interface_utils.h +++ b/libs/binder/ndk/include_cpp/android/binder_interface_utils.h @@ -30,7 +30,7 @@ #include <android/binder_auto_utils.h> #include <android/binder_ibinder.h> -#if defined(__ANDROID_VENDOR__) +#if defined(__ANDROID_VENDOR_API__) #include <android/llndk-versioning.h> #elif !defined(API_LEVEL_AT_LEAST) #if defined(__BIONIC__) @@ -39,7 +39,7 @@ #else #define API_LEVEL_AT_LEAST(sdk_api_level, vendor_api_level) (true) #endif // __BIONIC__ -#endif // __ANDROID_VENDOR__ +#endif // __ANDROID_VENDOR_API__ #if __has_include(<android/binder_shell.h>) #include <android/binder_shell.h> @@ -297,9 +297,7 @@ AIBinder_Class* ICInterface::defineClass(const char* interfaceDescriptor, } #endif -// TODO(b/368559337): fix versioning on product partition -#if !defined(__ANDROID_PRODUCT__) && \ - (defined(__ANDROID_UNAVAILABLE_SYMBOLS_ARE_WEAK__) || __ANDROID_API__ >= 36) +#if defined(__ANDROID_UNAVAILABLE_SYMBOLS_ARE_WEAK__) || __ANDROID_API__ >= 36 if API_LEVEL_AT_LEAST (36, 202504) { if (codeToFunction != nullptr) { AIBinder_Class_setTransactionCodeToFunctionNameMap(clazz, codeToFunction, diff --git a/libs/binder/ndk/include_cpp/android/persistable_bundle_aidl.h b/libs/binder/ndk/include_cpp/android/persistable_bundle_aidl.h index c1d0e9f9fe..83976b3771 100644 --- a/libs/binder/ndk/include_cpp/android/persistable_bundle_aidl.h +++ b/libs/binder/ndk/include_cpp/android/persistable_bundle_aidl.h @@ -22,8 +22,8 @@ #include <set> #include <sstream> -// Include llndk-versioning.h only for vendor build as it is not available for NDK headers. -#if defined(__ANDROID_VENDOR__) +// Include llndk-versioning.h only for non-system build as it is not available for NDK headers. +#if defined(__ANDROID_VENDOR_API__) #include <android/llndk-versioning.h> #elif !defined(API_LEVEL_AT_LEAST) #if defined(__BIONIC__) @@ -32,7 +32,7 @@ #else #define API_LEVEL_AT_LEAST(sdk_api_level, vendor_api_level) (true) #endif // __BIONIC__ -#endif // __ANDROID_VENDOR__ +#endif // __ANDROID_VENDOR_API__ namespace aidl::android::os { diff --git a/libs/binder/ndk/include_ndk/android/binder_ibinder.h b/libs/binder/ndk/include_ndk/android/binder_ibinder.h index 2f6c4e3642..bd46c473b4 100644 --- a/libs/binder/ndk/include_ndk/android/binder_ibinder.h +++ b/libs/binder/ndk/include_ndk/android/binder_ibinder.h @@ -224,8 +224,10 @@ void AIBinder_Class_setOnDump(AIBinder_Class* clazz, AIBinder_onDump onDump) __I * * Trace messages will use the provided names instead of bare integer codes when set. If not set by * this function, trace messages will only be identified by the bare code. This should be called one - * time during clazz initialization. clazz and transactionCodeToFunctionMap should have same - * lifetime. Resetting/clearing the transactionCodeToFunctionMap is not allowed. + * time during clazz initialization. clazz is defined using AIBinder_Class_define and + * transactionCodeToFunctionMap should have same scope as clazz. Resetting/clearing the + * transactionCodeToFunctionMap is not allowed. Passing null for either clazz or + * transactionCodeToFunctionMap will abort. * * Available since API level 36. * @@ -236,9 +238,6 @@ void AIBinder_Class_setOnDump(AIBinder_Class* clazz, AIBinder_onDump onDump) __I * contiguous, and this is required for maximum memory efficiency. * You can use nullptr if certain transaction codes are not used. Lifetime should be same as clazz. * \param length number of elements in the transactionCodeToFunctionMap - * - * \return true if setting codeToFunction to clazz is successful. return false if clazz or - * codeToFunction is nullptr. */ void AIBinder_Class_setTransactionCodeToFunctionNameMap(AIBinder_Class* clazz, const char** transactionCodeToFunctionMap, @@ -257,7 +256,8 @@ void AIBinder_Class_setTransactionCodeToFunctionNameMap(AIBinder_Class* clazz, * \param transactionCode transaction_code_t for which function name is requested. * * \return function name in form of const char* if transaction code is valid for given class. - * if transaction code is invalid or transactionCodeToFunctionMap is not set, nullptr is returned + * The value returned is valid for the lifetime of clazz. if transaction code is invalid or + * transactionCodeToFunctionMap is not set, nullptr is returned. */ const char* AIBinder_Class_getFunctionName(AIBinder_Class* clazz, transaction_code_t code) __INTRODUCED_IN(36); diff --git a/libs/binder/ndk/include_platform/android/binder_rpc.h b/libs/binder/ndk/include_platform/android/binder_rpc.h index 4c5471ff70..71325541cd 100644 --- a/libs/binder/ndk/include_platform/android/binder_rpc.h +++ b/libs/binder/ndk/include_platform/android/binder_rpc.h @@ -22,6 +22,22 @@ __BEGIN_DECLS /** + * @defgroup ABinderRpc Binder RPC + * + * This set of APIs makes it possible for a process to use the AServiceManager + * APIs to get binder objects for services that are available over sockets + * instead of the traditional kernel binder with the extra ServiceManager + * process. + * + * These APIs are used to supply libbinder with enough information to create + * and manage the socket connections underneath the ServiceManager APIs so the + * clients do not need to know the service implementation details or what + * transport they use for communication. + * + * @{ + */ + +/** * This represents an IAccessor implementation from libbinder that is * responsible for providing a pre-connected socket file descriptor for a * specific service. The service is an RpcServer and the pre-connected socket is @@ -66,7 +82,8 @@ typedef struct ABinderRpc_ConnectionInfo ABinderRpc_ConnectionInfo; * libbinder. * * \param instance name of the service like - * `android.hardware.vibrator.IVibrator/default` + * `android.hardware.vibrator.IVibrator/default`. This string must remain + * valid and unchanged for the duration of this function call. * \param data the data that was associated with this instance when the callback * was registered. * \return The ABinderRpc_Accessor associated with the service `instance`. This @@ -103,13 +120,15 @@ typedef void (*ABinderRpc_AccessorProviderUserData_deleteCallback)(void* _Nullab * instance is being registered that was previously registered, this call * will fail and the ABinderRpc_AccessorProviderUserData_deleteCallback * will be called to clean up the data. + * This array of strings must remain valid and unchanged for the duration + * of this function call. * \param number of instances in the instances array. * \param data pointer that is passed to the ABinderRpc_AccessorProvider callback. * IMPORTANT: The ABinderRpc_AccessorProvider now OWNS that object that data * points to. It can be used as necessary in the callback. The data MUST * remain valid for the lifetime of the provider callback. * Do not attempt to give ownership of the same object to different - * providers throguh multiple calls to this function because the first + * providers through multiple calls to this function because the first * one to be deleted will call the onDelete callback. * \param onDelete callback used to delete the objects that `data` points to. * This is called after ABinderRpc_AccessorProvider is guaranteed to never be @@ -151,8 +170,9 @@ void ABinderRpc_unregisterAccessorProvider(ABinderRpc_AccessorProvider* _Nonnull * connect to a socket that a given service is listening on. This is needed to * create an ABinderRpc_Accessor so it can connect to these services. * - * \param instance name of the service to connect to - * \param data userdata for this callback. The pointer is provided in + * \param instance name of the service to connect to. This string must remain + * valid and unchanged for the duration of this function call. + * \param data user data for this callback. The pointer is provided in * ABinderRpc_Accessor_new. * \return ABinderRpc_ConnectionInfo with socket connection information for `instance` */ @@ -177,7 +197,9 @@ typedef void (*ABinderRpc_ConnectionInfoProviderUserData_delete)(void* _Nullable * that can use the info from the ABinderRpc_ConnectionInfoProvider to connect to a * socket that the service with `instance` name is listening to. * - * \param instance name of the service that is listening on the socket + * \param instance name of the service that is listening on the socket. This + * string must remain valid and unchanged for the duration of this + * function call. * \param provider callback that can get the socket connection information for the * instance. This connection information may be dynamic, so the * provider will be called any time a new connection is required. @@ -219,18 +241,24 @@ AIBinder* _Nullable ABinderRpc_Accessor_asBinder(ABinderRpc_Accessor* _Nonnull a /** * Return the ABinderRpc_Accessor associated with an AIBinder. The instance must match - * the ABinderRpc_Accessor implementation, and the AIBinder must a proxy binder for a - * remote service (ABpBinder). - * This can be used when receivng an AIBinder from another process that the + * the ABinderRpc_Accessor implementation. + * This can be used when receiving an AIBinder from another process that the * other process obtained from ABinderRpc_Accessor_asBinder. * * \param instance name of the service that the Accessor is responsible for. - * \param accessorBinder proxy binder from another processes ABinderRpc_Accessor. + * This string must remain valid and unchanged for the duration of this + * function call. + * \param accessorBinder proxy binder from another process's ABinderRpc_Accessor. + * This function preserves the refcount of this binder object and the + * caller still owns it. * \return ABinderRpc_Accessor representing the other processes ABinderRpc_Accessor - * implementation. This function does not take ownership of the - * ABinderRpc_Accessor (so the caller needs to delete with - * ABinderRpc_Accessor_delete), and it preserves the recount of the bidner - * object. + * implementation. The caller owns this ABinderRpc_Accessor instance and + * is responsible for deleting it with ABinderRpc_Accessor_delete or + * passing ownership of it elsewhere, like returning it through + * ABinderRpc_AccessorProvider_getAccessorCallback. + * nullptr on error when the accessorBinder is not a valid binder from + * an IAccessor implementation or the IAccessor implementation is not + * associated with the provided instance. */ ABinderRpc_Accessor* _Nullable ABinderRpc_Accessor_fromBinder(const char* _Nonnull instance, AIBinder* _Nonnull accessorBinder) @@ -258,4 +286,6 @@ ABinderRpc_ConnectionInfo* _Nullable ABinderRpc_ConnectionInfo_new(const sockadd */ void ABinderRpc_ConnectionInfo_delete(ABinderRpc_ConnectionInfo* _Nonnull info) __INTRODUCED_IN(36); +/** @} */ + __END_DECLS diff --git a/libs/binder/rust/Android.bp b/libs/binder/rust/Android.bp index 2deb25457e..4545d7bb2e 100644 --- a/libs/binder/rust/Android.bp +++ b/libs/binder/rust/Android.bp @@ -15,6 +15,8 @@ rust_library { "libbinder_ndk_sys", "libdowncast_rs", "liblibc", + "liblog_rust", + "libnix", ], host_supported: true, vendor_available: true, @@ -79,6 +81,9 @@ rust_library { shared_libs: [ "libbinder_ndk", ], + rustlibs: [ + "liblibc", + ], host_supported: true, vendor_available: true, product_available: true, @@ -129,9 +134,18 @@ rust_bindgen { // rustified "libbinder_ndk_bindgen_flags.txt", ], + bindgen_flags: [ + "--blocklist-type", + "sockaddr", + "--raw-line", + "use libc::sockaddr;", + ], shared_libs: [ "libbinder_ndk", ], + rustlibs: [ + "liblibc", + ], host_supported: true, vendor_available: true, product_available: true, @@ -185,6 +199,8 @@ rust_test { "libbinder_ndk_sys", "libdowncast_rs", "liblibc", + "liblog_rust", + "libnix", ], } @@ -196,4 +212,7 @@ rust_test { auto_gen_config: true, clippy_lints: "none", lints: "none", + rustlibs: [ + "liblibc", + ], } diff --git a/libs/binder/rust/src/binder.rs b/libs/binder/rust/src/binder.rs index 9a252b853b..23026e593c 100644 --- a/libs/binder/rust/src/binder.rs +++ b/libs/binder/rust/src/binder.rs @@ -136,6 +136,31 @@ impl TryFrom<i32> for Stability { } } +/// Same as `Stability`, but in the form of a trait. Used when the stability should be encoded in +/// the type. +/// +/// When/if the `adt_const_params` Rust feature is stabilized, this could be replace by using +/// `Stability` directly with const generics. +pub trait StabilityType { + /// The `Stability` represented by this type. + const VALUE: Stability; +} + +/// `Stability::Local`. +#[derive(Debug)] +pub enum LocalStabilityType {} +/// `Stability::Vintf`. +#[derive(Debug)] +pub enum VintfStabilityType {} + +impl StabilityType for LocalStabilityType { + const VALUE: Stability = Stability::Local; +} + +impl StabilityType for VintfStabilityType { + const VALUE: Stability = Stability::Vintf; +} + /// A local service that can be remotable via Binder. /// /// An object that implement this interface made be made into a Binder service diff --git a/libs/binder/rust/src/lib.rs b/libs/binder/rust/src/lib.rs index e70f4f0232..0e8e388c2b 100644 --- a/libs/binder/rust/src/lib.rs +++ b/libs/binder/rust/src/lib.rs @@ -104,6 +104,8 @@ mod proxy; mod service; #[cfg(not(trusty))] mod state; +#[cfg(not(any(android_vendor, android_vndk)))] +mod system_only; use binder_ndk_sys as sys; @@ -120,6 +122,8 @@ pub use service::{ }; #[cfg(not(trusty))] pub use state::{ProcessState, ThreadState}; +#[cfg(not(any(android_vendor, android_vndk)))] +pub use system_only::{Accessor, ConnectionInfo}; /// Binder result containing a [`Status`] on error. pub type Result<T> = std::result::Result<T, Status>; @@ -128,9 +132,10 @@ pub type Result<T> = std::result::Result<T, Status>; /// without AIDL. pub mod binder_impl { pub use crate::binder::{ - IBinderInternal, InterfaceClass, Remotable, Stability, ToAsyncInterface, ToSyncInterface, - TransactionCode, TransactionFlags, FIRST_CALL_TRANSACTION, FLAG_CLEAR_BUF, FLAG_ONEWAY, - FLAG_PRIVATE_LOCAL, LAST_CALL_TRANSACTION, + IBinderInternal, InterfaceClass, LocalStabilityType, Remotable, Stability, StabilityType, + ToAsyncInterface, ToSyncInterface, TransactionCode, TransactionFlags, VintfStabilityType, + FIRST_CALL_TRANSACTION, FLAG_CLEAR_BUF, FLAG_ONEWAY, FLAG_PRIVATE_LOCAL, + LAST_CALL_TRANSACTION, }; pub use crate::binder_async::BinderAsyncRuntime; pub use crate::error::status_t; diff --git a/libs/binder/rust/src/parcel/parcelable_holder.rs b/libs/binder/rust/src/parcel/parcelable_holder.rs index f90611361f..87b42ab68d 100644 --- a/libs/binder/rust/src/parcel/parcelable_holder.rs +++ b/libs/binder/rust/src/parcel/parcelable_holder.rs @@ -15,6 +15,7 @@ */ use crate::binder::Stability; +use crate::binder::StabilityType; use crate::error::StatusCode; use crate::parcel::{ BorrowedParcel, Deserialize, Parcel, Parcelable, Serialize, NON_NULL_PARCELABLE_FLAG, @@ -60,7 +61,7 @@ enum ParcelableHolderData { /// `Send` nor `Sync`), mainly because it internally contains /// a `Parcel` which in turn is not thread-safe. #[derive(Debug)] -pub struct ParcelableHolder { +pub struct ParcelableHolder<STABILITY: StabilityType> { // This is a `Mutex` because of `get_parcelable` // which takes `&self` for consistency with C++. // We could make `get_parcelable` take a `&mut self` @@ -68,13 +69,17 @@ pub struct ParcelableHolder { // improvement, but then callers would require a mutable // `ParcelableHolder` even for that getter method. data: Mutex<ParcelableHolderData>, - stability: Stability, + + _stability_phantom: std::marker::PhantomData<STABILITY>, } -impl ParcelableHolder { +impl<STABILITY: StabilityType> ParcelableHolder<STABILITY> { /// Construct a new `ParcelableHolder` with the given stability. - pub fn new(stability: Stability) -> Self { - Self { data: Mutex::new(ParcelableHolderData::Empty), stability } + pub fn new() -> Self { + Self { + data: Mutex::new(ParcelableHolderData::Empty), + _stability_phantom: Default::default(), + } } /// Reset the contents of this `ParcelableHolder`. @@ -91,7 +96,7 @@ impl ParcelableHolder { where T: Any + Parcelable + ParcelableMetadata + std::fmt::Debug + Send + Sync, { - if self.stability > p.get_stability() { + if STABILITY::VALUE > p.get_stability() { return Err(StatusCode::BAD_VALUE); } @@ -157,30 +162,36 @@ impl ParcelableHolder { /// Return the stability value of this object. pub fn get_stability(&self) -> Stability { - self.stability + STABILITY::VALUE + } +} + +impl<STABILITY: StabilityType> Default for ParcelableHolder<STABILITY> { + fn default() -> Self { + Self::new() } } -impl Clone for ParcelableHolder { - fn clone(&self) -> ParcelableHolder { +impl<STABILITY: StabilityType> Clone for ParcelableHolder<STABILITY> { + fn clone(&self) -> Self { ParcelableHolder { data: Mutex::new(self.data.lock().unwrap().clone()), - stability: self.stability, + _stability_phantom: Default::default(), } } } -impl Serialize for ParcelableHolder { +impl<STABILITY: StabilityType> Serialize for ParcelableHolder<STABILITY> { fn serialize(&self, parcel: &mut BorrowedParcel<'_>) -> Result<(), StatusCode> { parcel.write(&NON_NULL_PARCELABLE_FLAG)?; self.write_to_parcel(parcel) } } -impl Deserialize for ParcelableHolder { +impl<STABILITY: StabilityType> Deserialize for ParcelableHolder<STABILITY> { type UninitType = Self; fn uninit() -> Self::UninitType { - Self::new(Default::default()) + Self::new() } fn from_init(value: Self) -> Self::UninitType { value @@ -191,16 +202,16 @@ impl Deserialize for ParcelableHolder { if status == NULL_PARCELABLE_FLAG { Err(StatusCode::UNEXPECTED_NULL) } else { - let mut parcelable = ParcelableHolder::new(Default::default()); + let mut parcelable = Self::new(); parcelable.read_from_parcel(parcel)?; Ok(parcelable) } } } -impl Parcelable for ParcelableHolder { +impl<STABILITY: StabilityType> Parcelable for ParcelableHolder<STABILITY> { fn write_to_parcel(&self, parcel: &mut BorrowedParcel<'_>) -> Result<(), StatusCode> { - parcel.write(&self.stability)?; + parcel.write(&STABILITY::VALUE)?; let mut data = self.data.lock().unwrap(); match *data { @@ -236,7 +247,7 @@ impl Parcelable for ParcelableHolder { } fn read_from_parcel(&mut self, parcel: &BorrowedParcel<'_>) -> Result<(), StatusCode> { - if self.stability != parcel.read()? { + if self.get_stability() != parcel.read()? { return Err(StatusCode::BAD_VALUE); } diff --git a/libs/binder/rust/src/system_only.rs b/libs/binder/rust/src/system_only.rs new file mode 100644 index 0000000000..a91d84d865 --- /dev/null +++ b/libs/binder/rust/src/system_only.rs @@ -0,0 +1,187 @@ +/* + * 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. + */ + +use crate::proxy::SpIBinder; +use crate::sys; + +use std::ffi::{c_void, CStr, CString}; +use std::os::raw::c_char; + +use libc::sockaddr; +use nix::sys::socket::{SockaddrLike, UnixAddr, VsockAddr}; +use std::sync::Arc; +use std::{fmt, ptr}; + +/// Rust wrapper around ABinderRpc_Accessor objects for RPC binder service management. +/// +/// Dropping the `Accessor` will drop the underlying object and the binder it owns. +pub struct Accessor { + accessor: *mut sys::ABinderRpc_Accessor, +} + +impl fmt::Debug for Accessor { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + write!(f, "ABinderRpc_Accessor({:p})", self.accessor) + } +} + +/// Socket connection info required for libbinder to connect to a service. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub enum ConnectionInfo { + /// For vsock connection + Vsock(VsockAddr), + /// For unix domain socket connection + Unix(UnixAddr), +} + +/// Safety: A `Accessor` is a wrapper around `ABinderRpc_Accessor` which is +/// `Sync` and `Send`. As +/// `ABinderRpc_Accessor` is threadsafe, this structure is too. +/// The Fn owned the Accessor has `Sync` and `Send` properties +unsafe impl Send for Accessor {} + +/// Safety: A `Accessor` is a wrapper around `ABinderRpc_Accessor` which is +/// `Sync` and `Send`. As `ABinderRpc_Accessor` is threadsafe, this structure is too. +/// The Fn owned the Accessor has `Sync` and `Send` properties +unsafe impl Sync for Accessor {} + +impl Accessor { + /// Create a new accessor that will call the given callback when its + /// connection info is required. + /// The callback object and all objects it captures are owned by the Accessor + /// and will be deleted some time after the Accessor is Dropped. If the callback + /// is being called when the Accessor is Dropped, the callback will not be deleted + /// immediately. + pub fn new<F>(instance: &str, callback: F) -> Accessor + where + F: Fn(&str) -> Option<ConnectionInfo> + Send + Sync + 'static, + { + let callback: *mut c_void = Arc::into_raw(Arc::new(callback)) as *mut c_void; + let inst = CString::new(instance).unwrap(); + + // Safety: The function pointer is a valid connection_info callback. + // This call returns an owned `ABinderRpc_Accessor` pointer which + // must be destroyed via `ABinderRpc_Accessor_delete` when no longer + // needed. + // When the underlying ABinderRpc_Accessor is deleted, it will call + // the cookie_decr_refcount callback to release its strong ref. + let accessor = unsafe { + sys::ABinderRpc_Accessor_new( + inst.as_ptr(), + Some(Self::connection_info::<F>), + callback, + Some(Self::cookie_decr_refcount::<F>), + ) + }; + + Accessor { accessor } + } + + /// Get the underlying binder for this Accessor for when it needs to be either + /// registered with service manager or sent to another process. + pub fn as_binder(&self) -> Option<SpIBinder> { + // Safety: `ABinderRpc_Accessor_asBinder` returns either a null pointer or a + // valid pointer to an owned `AIBinder`. Either of these values is safe to + // pass to `SpIBinder::from_raw`. + unsafe { SpIBinder::from_raw(sys::ABinderRpc_Accessor_asBinder(self.accessor)) } + } + + /// Callback invoked from C++ when the connection info is needed. + /// + /// # Safety + /// + /// The `instance` parameter must be a non-null pointer to a valid C string for + /// CStr::from_ptr. The memory must contain a valid null terminator at the end of + /// the string within isize::MAX from the pointer. The memory must not be mutated for + /// the duration of this function call and must be valid for reads from the pointer + /// to the null terminator. + /// The `cookie` parameter must be the cookie for an `Arc<F>` and + /// the caller must hold a ref-count to it. + unsafe extern "C" fn connection_info<F>( + instance: *const c_char, + cookie: *mut c_void, + ) -> *mut binder_ndk_sys::ABinderRpc_ConnectionInfo + where + F: Fn(&str) -> Option<ConnectionInfo> + Send + Sync + 'static, + { + if cookie.is_null() || instance.is_null() { + log::error!("Cookie({cookie:p}) or instance({instance:p}) is null!"); + return ptr::null_mut(); + } + // Safety: The caller promises that `cookie` is for an Arc<F>. + let callback = unsafe { (cookie as *const F).as_ref().unwrap() }; + + // Safety: The caller in libbinder_ndk will have already verified this is a valid + // C string + let inst = unsafe { + match CStr::from_ptr(instance).to_str() { + Ok(s) => s, + Err(err) => { + log::error!("Failed to get a valid C string! {err:?}"); + return ptr::null_mut(); + } + } + }; + + let connection = match callback(inst) { + Some(con) => con, + None => { + return ptr::null_mut(); + } + }; + + match connection { + ConnectionInfo::Vsock(addr) => { + // Safety: The sockaddr is being copied in the NDK API + unsafe { sys::ABinderRpc_ConnectionInfo_new(addr.as_ptr(), addr.len()) } + } + ConnectionInfo::Unix(addr) => { + // Safety: The sockaddr is being copied in the NDK API + // The cast is from sockaddr_un* to sockaddr*. + unsafe { + sys::ABinderRpc_ConnectionInfo_new(addr.as_ptr() as *const sockaddr, addr.len()) + } + } + } + } + + /// Callback that decrements the ref-count. + /// This is invoked from C++ when a binder is unlinked. + /// + /// # Safety + /// + /// The `cookie` parameter must be the cookie for an `Arc<F>` and + /// the owner must give up a ref-count to it. + unsafe extern "C" fn cookie_decr_refcount<F>(cookie: *mut c_void) + where + F: Fn(&str) -> Option<ConnectionInfo> + Send + Sync + 'static, + { + // Safety: The caller promises that `cookie` is for an Arc<F>. + unsafe { Arc::decrement_strong_count(cookie as *const F) }; + } +} + +impl Drop for Accessor { + fn drop(&mut self) { + // Safety: `self.accessor` is always a valid, owned + // `ABinderRpc_Accessor` pointer returned by + // `ABinderRpc_Accessor_new` when `self` was created. This delete + // method can only be called once when `self` is dropped. + unsafe { + sys::ABinderRpc_Accessor_delete(self.accessor); + } + } +} diff --git a/libs/binder/rust/sys/BinderBindings.hpp b/libs/binder/rust/sys/BinderBindings.hpp index 65fa2ca905..bd666fe0d7 100644 --- a/libs/binder/rust/sys/BinderBindings.hpp +++ b/libs/binder/rust/sys/BinderBindings.hpp @@ -20,6 +20,7 @@ #include <android/binder_parcel.h> #include <android/binder_parcel_platform.h> #include <android/binder_process.h> +#include <android/binder_rpc.h> #include <android/binder_shell.h> #include <android/binder_stability.h> #include <android/binder_status.h> diff --git a/libs/binder/rust/tests/integration.rs b/libs/binder/rust/tests/integration.rs index 5359832da1..bdb7e4a8d8 100644 --- a/libs/binder/rust/tests/integration.rs +++ b/libs/binder/rust/tests/integration.rs @@ -384,8 +384,8 @@ mod tests { use std::time::Duration; use binder::{ - BinderFeatures, DeathRecipient, FromIBinder, IBinder, Interface, SpIBinder, StatusCode, - Strong, + Accessor, BinderFeatures, DeathRecipient, FromIBinder, IBinder, Interface, SpIBinder, + StatusCode, Strong, }; // Import from impl API for testing only, should not be necessary as long as // you are using AIDL. @@ -908,6 +908,43 @@ mod tests { assert_eq!(service.test().unwrap(), service_name); } + struct ToBeDeleted { + deleted: Arc<AtomicBool>, + } + + impl Drop for ToBeDeleted { + fn drop(&mut self) { + assert!(!self.deleted.load(Ordering::Relaxed)); + self.deleted.store(true, Ordering::Relaxed); + } + } + + #[test] + fn test_accessor_callback_destruction() { + let deleted: Arc<AtomicBool> = Arc::new(AtomicBool::new(false)); + { + let accessor: Accessor; + { + let helper = ToBeDeleted { deleted: deleted.clone() }; + let get_connection_info = move |_instance: &str| { + // Capture this object so we can see it get destructed + // after the parent scope + let _ = &helper; + None + }; + accessor = Accessor::new("foo.service", get_connection_info); + } + + match accessor.as_binder() { + Some(_) => { + assert!(!deleted.load(Ordering::Relaxed)); + } + None => panic!("failed to get that accessor binder"), + } + } + assert!(deleted.load(Ordering::Relaxed)); + } + #[tokio::test] async fn reassociate_rust_binder_async() { let service_name = "testing_service"; diff --git a/libs/binder/rust/tests/parcel_fuzzer/parcel_fuzzer.rs b/libs/binder/rust/tests/parcel_fuzzer/parcel_fuzzer.rs index ce0f742934..ee20a22345 100644 --- a/libs/binder/rust/tests/parcel_fuzzer/parcel_fuzzer.rs +++ b/libs/binder/rust/tests/parcel_fuzzer/parcel_fuzzer.rs @@ -21,7 +21,8 @@ mod read_utils; use crate::read_utils::READ_FUNCS; use binder::binder_impl::{ - Binder, BorrowedParcel, IBinderInternal, Parcel, Stability, TransactionCode, + Binder, BorrowedParcel, IBinderInternal, LocalStabilityType, Parcel, TransactionCode, + VintfStabilityType, }; use binder::{ declare_binder_interface, BinderFeatures, Interface, Parcelable, ParcelableHolder, SpIBinder, @@ -121,13 +122,15 @@ fn do_read_fuzz(read_operations: Vec<ReadOperation>, data: &[u8]) { } ReadOperation::ReadParcelableHolder { is_vintf } => { - let stability = if is_vintf { Stability::Vintf } else { Stability::Local }; - let mut holder: ParcelableHolder = ParcelableHolder::new(stability); - match holder.read_from_parcel(parcel.borrowed_ref()) { - Ok(result) => result, - Err(err) => { - println!("error occurred while reading from parcel: {:?}", err) - } + let result = if is_vintf { + ParcelableHolder::<VintfStabilityType>::new() + .read_from_parcel(parcel.borrowed_ref()) + } else { + ParcelableHolder::<LocalStabilityType>::new() + .read_from_parcel(parcel.borrowed_ref()) + }; + if let Err(e) = result { + println!("error occurred while reading from parcel: {e:?}") } } diff --git a/libs/binder/tests/binderCacheUnitTest.cpp b/libs/binder/tests/binderCacheUnitTest.cpp index 482d197688..c5ad79391c 100644 --- a/libs/binder/tests/binderCacheUnitTest.cpp +++ b/libs/binder/tests/binderCacheUnitTest.cpp @@ -149,7 +149,16 @@ TEST_F(LibbinderCacheTest, RemoveFromCacheOnServerDeath) { EXPECT_EQ(OK, mServiceManager->addService(kCachedServiceName, binder2)); // Confirm that new service is returned instead of old. - sp<IBinder> result2 = mServiceManager->checkService(kCachedServiceName); + int retry_count = 20; + sp<IBinder> result2; + do { + std::this_thread::sleep_for(std::chrono::milliseconds(50)); + if (retry_count-- == 0) { + break; + } + result2 = mServiceManager->checkService(kCachedServiceName); + } while (result2 != binder2); + ASSERT_EQ(binder2, result2); } diff --git a/libs/binder/tests/parcel_fuzzer/Android.bp b/libs/binder/tests/parcel_fuzzer/Android.bp index fbab8f08a9..cac054eb1b 100644 --- a/libs/binder/tests/parcel_fuzzer/Android.bp +++ b/libs/binder/tests/parcel_fuzzer/Android.bp @@ -39,6 +39,7 @@ cc_fuzz { "smoreland@google.com", "waghpawan@google.com", ], + triage_assignee: "smoreland@google.com", use_for_presubmit: true, }, diff --git a/libs/binder/tests/parcel_fuzzer/binder.cpp b/libs/binder/tests/parcel_fuzzer/binder.cpp index e378b864f7..07f0143767 100644 --- a/libs/binder/tests/parcel_fuzzer/binder.cpp +++ b/libs/binder/tests/parcel_fuzzer/binder.cpp @@ -25,6 +25,8 @@ #include <binder/ParcelableHolder.h> #include <binder/PersistableBundle.h> #include <binder/Status.h> +#include <fuzzbinder/random_binder.h> +#include <fuzzbinder/random_fd.h> #include <utils/Flattenable.h> #include "../../Utils.h" @@ -115,14 +117,6 @@ std::vector<ParcelRead<::android::Parcel>> BINDER_PARCEL_READ_FUNCTIONS { p.setDataPosition(pos); FUZZ_LOG() << "setDataPosition done"; }, - [] (const ::android::Parcel& p, FuzzedDataProvider& provider) { - size_t len = provider.ConsumeIntegralInRange<size_t>(0, 1024); - std::vector<uint8_t> bytes = provider.ConsumeBytes<uint8_t>(len); - FUZZ_LOG() << "about to setData: " <<(bytes.data() ? HexString(bytes.data(), bytes.size()) : "null"); - // TODO: allow all read and write operations - (*const_cast<::android::Parcel*>(&p)).setData(bytes.data(), bytes.size()); - FUZZ_LOG() << "setData done"; - }, PARCEL_READ_NO_STATUS(size_t, allowFds), PARCEL_READ_NO_STATUS(size_t, hasFileDescriptors), PARCEL_READ_NO_STATUS(std::vector<android::sp<android::IBinder>>, debugReadAllStrongBinders), @@ -404,5 +398,113 @@ std::vector<ParcelRead<::android::Parcel>> BINDER_PARCEL_READ_FUNCTIONS { FUZZ_LOG() << " toString() result: " << toString; }, }; + +std::vector<ParcelWrite<::android::Parcel>> BINDER_PARCEL_WRITE_FUNCTIONS { + [] (::android::Parcel& p, FuzzedDataProvider& provider, android::RandomParcelOptions* /*options*/) { + FUZZ_LOG() << "about to call setDataSize"; + size_t len = provider.ConsumeIntegralInRange<size_t>(0, 1024); + p.setDataSize(len); + }, + [] (::android::Parcel& p, FuzzedDataProvider& provider, android::RandomParcelOptions* /*options*/) { + FUZZ_LOG() << "about to call setDataCapacity"; + size_t len = provider.ConsumeIntegralInRange<size_t>(0, 1024); + p.setDataCapacity(len); + }, + [] (::android::Parcel& p, FuzzedDataProvider& provider, android::RandomParcelOptions* /*options*/) { + FUZZ_LOG() << "about to call setData"; + size_t len = provider.ConsumeIntegralInRange<size_t>(0, 1024); + std::vector<uint8_t> bytes = provider.ConsumeBytes<uint8_t>(len); + p.setData(bytes.data(), bytes.size()); + }, + [] (::android::Parcel& p, FuzzedDataProvider& provider, android::RandomParcelOptions* options) { + FUZZ_LOG() << "about to call appendFrom"; + + std::vector<uint8_t> bytes = provider.ConsumeBytes<uint8_t>(provider.ConsumeIntegralInRange<size_t>(0, 4096)); + ::android::Parcel p2; + fillRandomParcel(&p2, FuzzedDataProvider(bytes.data(), bytes.size()), options); + + int32_t start = provider.ConsumeIntegral<int32_t>(); + int32_t len = provider.ConsumeIntegral<int32_t>(); + p.appendFrom(&p2, start, len); + }, + [] (::android::Parcel& p, FuzzedDataProvider& provider, android::RandomParcelOptions* /*options*/) { + FUZZ_LOG() << "about to call pushAllowFds"; + bool val = provider.ConsumeBool(); + p.pushAllowFds(val); + }, + [] (::android::Parcel& p, FuzzedDataProvider& provider, android::RandomParcelOptions* /*options*/) { + FUZZ_LOG() << "about to call restoreAllowFds"; + bool val = provider.ConsumeBool(); + p.restoreAllowFds(val); + }, + // markForBinder - covered by fillRandomParcel, aborts if called multiple times + // markForRpc - covered by fillRandomParcel, aborts if called multiple times + [] (::android::Parcel& p, FuzzedDataProvider& provider, android::RandomParcelOptions* /*options*/) { + FUZZ_LOG() << "about to call writeInterfaceToken"; + std::string interface = provider.ConsumeRandomLengthString(); + p.writeInterfaceToken(android::String16(interface.c_str())); + }, + [] (::android::Parcel& p, FuzzedDataProvider& provider, android::RandomParcelOptions* /*options*/) { + FUZZ_LOG() << "about to call setEnforceNoDataAvail"; + p.setEnforceNoDataAvail(provider.ConsumeBool()); + }, + [] (::android::Parcel& p, FuzzedDataProvider& /* provider */, android::RandomParcelOptions* /*options*/) { + FUZZ_LOG() << "about to call setServiceFuzzing"; + p.setServiceFuzzing(); + }, + [] (::android::Parcel& p, FuzzedDataProvider& /* provider */, android::RandomParcelOptions* /*options*/) { + FUZZ_LOG() << "about to call freeData"; + p.freeData(); + }, + [] (::android::Parcel& p, FuzzedDataProvider& provider, android::RandomParcelOptions* /*options*/) { + FUZZ_LOG() << "about to call write"; + size_t len = provider.ConsumeIntegralInRange<size_t>(0, 256); + std::vector<uint8_t> bytes = provider.ConsumeBytes<uint8_t>(len); + p.write(bytes.data(), bytes.size()); + }, + // write* - write functions all implemented by calling 'write' itself. + [] (::android::Parcel& p, FuzzedDataProvider& provider, android::RandomParcelOptions* options) { + FUZZ_LOG() << "about to call writeStrongBinder"; + + // TODO: this logic is somewhat duplicated with random parcel + android::sp<android::IBinder> binder; + if (provider.ConsumeBool() && options->extraBinders.size() > 0) { + binder = options->extraBinders.at( + provider.ConsumeIntegralInRange<size_t>(0, options->extraBinders.size() - 1)); + } else { + binder = android::getRandomBinder(&provider); + options->extraBinders.push_back(binder); + } + + p.writeStrongBinder(binder); + }, + [] (::android::Parcel& p, FuzzedDataProvider& /* provider */, android::RandomParcelOptions* /*options*/) { + FUZZ_LOG() << "about to call writeFileDescriptor (no ownership)"; + p.writeFileDescriptor(STDERR_FILENO, false /* takeOwnership */); + }, + [] (::android::Parcel& p, FuzzedDataProvider& provider, android::RandomParcelOptions* options) { + FUZZ_LOG() << "about to call writeFileDescriptor (take ownership)"; + std::vector<unique_fd> fds = android::getRandomFds(&provider); + if (fds.size() == 0) return; + + p.writeDupFileDescriptor(fds.at(0).get()); + options->extraFds.insert(options->extraFds.end(), + std::make_move_iterator(fds.begin() + 1), + std::make_move_iterator(fds.end())); + }, + // TODO: writeBlob + // TODO: writeDupImmutableBlobFileDescriptor + // TODO: writeObject (or make the API private more likely) + [] (::android::Parcel& p, FuzzedDataProvider& /* provider */, android::RandomParcelOptions* /*options*/) { + FUZZ_LOG() << "about to call writeNoException"; + p.writeNoException(); + }, + [] (::android::Parcel& p, FuzzedDataProvider& provider, android::RandomParcelOptions* /*options*/) { + FUZZ_LOG() << "about to call replaceCallingWorkSourceUid"; + uid_t uid = provider.ConsumeIntegral<uid_t>(); + p.replaceCallingWorkSourceUid(uid); + }, +}; + // clang-format on #pragma clang diagnostic pop diff --git a/libs/binder/tests/parcel_fuzzer/binder.h b/libs/binder/tests/parcel_fuzzer/binder.h index 0c51d68a37..b0ac140d3f 100644 --- a/libs/binder/tests/parcel_fuzzer/binder.h +++ b/libs/binder/tests/parcel_fuzzer/binder.h @@ -21,3 +21,4 @@ #include "parcel_fuzzer.h" extern std::vector<ParcelRead<::android::Parcel>> BINDER_PARCEL_READ_FUNCTIONS; +extern std::vector<ParcelWrite<::android::Parcel>> BINDER_PARCEL_WRITE_FUNCTIONS; diff --git a/libs/binder/tests/parcel_fuzzer/binder_ndk.cpp b/libs/binder/tests/parcel_fuzzer/binder_ndk.cpp index e3a337171f..3f8d71d1f9 100644 --- a/libs/binder/tests/parcel_fuzzer/binder_ndk.cpp +++ b/libs/binder/tests/parcel_fuzzer/binder_ndk.cpp @@ -20,8 +20,11 @@ #include "aidl/parcelables/GenericDataParcelable.h" #include "aidl/parcelables/SingleDataParcelable.h" +#include <android/binder_libbinder.h> #include <android/binder_parcel_utils.h> #include <android/binder_parcelable_utils.h> +#include <fuzzbinder/random_binder.h> +#include <fuzzbinder/random_fd.h> #include "util.h" @@ -211,16 +214,51 @@ std::vector<ParcelRead<NdkParcelAdapter>> BINDER_NDK_PARCEL_READ_FUNCTIONS{ binder_status_t status = AParcel_marshal(p.aParcel(), buffer, start, len); FUZZ_LOG() << "status: " << status; }, - [](const NdkParcelAdapter& /*p*/, FuzzedDataProvider& provider) { - FUZZ_LOG() << "about to unmarshal AParcel"; +}; +std::vector<ParcelWrite<NdkParcelAdapter>> BINDER_NDK_PARCEL_WRITE_FUNCTIONS{ + [] (NdkParcelAdapter& p, FuzzedDataProvider& provider, android::RandomParcelOptions* options) { + FUZZ_LOG() << "about to call AParcel_writeStrongBinder"; + + // TODO: this logic is somewhat duplicated with random parcel + android::sp<android::IBinder> binder; + if (provider.ConsumeBool() && options->extraBinders.size() > 0) { + binder = options->extraBinders.at( + provider.ConsumeIntegralInRange<size_t>(0, options->extraBinders.size() - 1)); + } else { + binder = android::getRandomBinder(&provider); + options->extraBinders.push_back(binder); + } + + ndk::SpAIBinder abinder = ndk::SpAIBinder(AIBinder_fromPlatformBinder(binder)); + AParcel_writeStrongBinder(p.aParcel(), abinder.get()); + }, + [] (NdkParcelAdapter& p, FuzzedDataProvider& provider, android::RandomParcelOptions* options) { + FUZZ_LOG() << "about to call AParcel_writeParcelFileDescriptor"; + + auto fds = android::getRandomFds(&provider); + if (fds.size() == 0) return; + + AParcel_writeParcelFileDescriptor(p.aParcel(), fds.at(0).get()); + options->extraFds.insert(options->extraFds.end(), + std::make_move_iterator(fds.begin() + 1), + std::make_move_iterator(fds.end())); + }, + // all possible data writes can be done as a series of 4-byte reads + [] (NdkParcelAdapter& p, FuzzedDataProvider& provider, android::RandomParcelOptions* /*options*/) { + FUZZ_LOG() << "about to call AParcel_writeInt32"; + int32_t val = provider.ConsumeIntegral<int32_t>(); + AParcel_writeInt32(p.aParcel(), val); + }, + [] (NdkParcelAdapter& p, FuzzedDataProvider& /* provider */, android::RandomParcelOptions* /*options*/) { + FUZZ_LOG() << "about to call AParcel_reset"; + AParcel_reset(p.aParcel()); + }, + [](NdkParcelAdapter& p, FuzzedDataProvider& provider, android::RandomParcelOptions* /*options*/) { + FUZZ_LOG() << "about to call AParcel_unmarshal"; size_t len = provider.ConsumeIntegralInRange<size_t>(0, provider.remaining_bytes()); - std::vector<uint8_t> parcelData = provider.ConsumeBytes<uint8_t>(len); - const uint8_t* buffer = parcelData.data(); - const size_t bufferLen = parcelData.size(); - NdkParcelAdapter adapter; - binder_status_t status = AParcel_unmarshal(adapter.aParcel(), buffer, bufferLen); + std::vector<uint8_t> data = provider.ConsumeBytes<uint8_t>(len); + binder_status_t status = AParcel_unmarshal(p.aParcel(), data.data(), data.size()); FUZZ_LOG() << "status: " << status; }, - }; // clang-format on diff --git a/libs/binder/tests/parcel_fuzzer/binder_ndk.h b/libs/binder/tests/parcel_fuzzer/binder_ndk.h index d19f25bc88..0c8b725400 100644 --- a/libs/binder/tests/parcel_fuzzer/binder_ndk.h +++ b/libs/binder/tests/parcel_fuzzer/binder_ndk.h @@ -50,3 +50,4 @@ private: }; extern std::vector<ParcelRead<NdkParcelAdapter>> BINDER_NDK_PARCEL_READ_FUNCTIONS; +extern std::vector<ParcelWrite<NdkParcelAdapter>> BINDER_NDK_PARCEL_WRITE_FUNCTIONS; diff --git a/libs/binder/tests/parcel_fuzzer/main.cpp b/libs/binder/tests/parcel_fuzzer/main.cpp index a57d07fb24..192f9d5dce 100644 --- a/libs/binder/tests/parcel_fuzzer/main.cpp +++ b/libs/binder/tests/parcel_fuzzer/main.cpp @@ -80,6 +80,7 @@ void doTransactFuzz(const char* backend, const sp<B>& binder, FuzzedDataProvider (void)binder->transact(code, data, &reply, flag); } +// start with a Parcel full of data (e.g. like you get from another process) template <typename P> void doReadFuzz(const char* backend, const std::vector<ParcelRead<P>>& reads, FuzzedDataProvider&& provider) { @@ -95,10 +96,10 @@ void doReadFuzz(const char* backend, const std::vector<ParcelRead<P>>& reads, RandomParcelOptions options; P p; - fillRandomParcel(&p, std::move(provider), &options); + fillRandomParcel(&p, std::move(provider), &options); // consumes provider // since we are only using a byte to index - CHECK(reads.size() <= 255) << reads.size(); + CHECK_LE(reads.size(), 255u) << reads.size(); FUZZ_LOG() << "backend: " << backend; FUZZ_LOG() << "input: " << HexString(p.data(), p.dataSize()); @@ -115,26 +116,29 @@ void doReadFuzz(const char* backend, const std::vector<ParcelRead<P>>& reads, } } -// Append two random parcels. template <typename P> -void doAppendFuzz(const char* backend, FuzzedDataProvider&& provider) { - int32_t start = provider.ConsumeIntegral<int32_t>(); - int32_t len = provider.ConsumeIntegral<int32_t>(); - - std::vector<uint8_t> bytes = provider.ConsumeBytes<uint8_t>( - provider.ConsumeIntegralInRange<size_t>(0, provider.remaining_bytes())); - - // same options so that FDs and binders could be shared in both Parcels +void doReadWriteFuzz(const char* backend, const std::vector<ParcelRead<P>>& reads, + const std::vector<ParcelWrite<P>>& writes, FuzzedDataProvider&& provider) { RandomParcelOptions options; + P p; - P p0, p1; - fillRandomParcel(&p0, FuzzedDataProvider(bytes.data(), bytes.size()), &options); - fillRandomParcel(&p1, std::move(provider), &options); + // since we are only using a byte to index + CHECK_LE(reads.size() + writes.size(), 255u) << reads.size(); FUZZ_LOG() << "backend: " << backend; - FUZZ_LOG() << "start: " << start << " len: " << len; - p0.appendFrom(&p1, start, len); + while (provider.remaining_bytes() > 0) { + uint8_t idx = provider.ConsumeIntegralInRange<uint8_t>(0, reads.size() + writes.size() - 1); + + FUZZ_LOG() << "Instruction " << idx << " avail: " << p.dataAvail() + << " pos: " << p.dataPosition() << " cap: " << p.dataCapacity(); + + if (idx < reads.size()) { + reads.at(idx)(p, provider); + } else { + writes.at(idx - reads.size())(p, provider, &options); + } + } } void* NothingClass_onCreate(void* args) { @@ -156,7 +160,7 @@ extern "C" int LLVMFuzzerTestOneInput(const uint8_t* data, size_t size) { FuzzedDataProvider provider = FuzzedDataProvider(data, size); - const std::function<void(FuzzedDataProvider &&)> fuzzBackend[] = { + const std::function<void(FuzzedDataProvider&&)> fuzzBackend[] = { [](FuzzedDataProvider&& provider) { doTransactFuzz< ::android::hardware::Parcel>("hwbinder", @@ -187,10 +191,14 @@ extern "C" int LLVMFuzzerTestOneInput(const uint8_t* data, size_t size) { std::move(provider)); }, [](FuzzedDataProvider&& provider) { - doAppendFuzz<::android::Parcel>("binder", std::move(provider)); + doReadWriteFuzz<::android::Parcel>("binder", BINDER_PARCEL_READ_FUNCTIONS, + BINDER_PARCEL_WRITE_FUNCTIONS, + std::move(provider)); }, [](FuzzedDataProvider&& provider) { - doAppendFuzz<NdkParcelAdapter>("binder_ndk", std::move(provider)); + doReadWriteFuzz<NdkParcelAdapter>("binder_ndk", BINDER_NDK_PARCEL_READ_FUNCTIONS, + BINDER_NDK_PARCEL_WRITE_FUNCTIONS, + std::move(provider)); }, }; diff --git a/libs/binder/tests/parcel_fuzzer/parcel_fuzzer.h b/libs/binder/tests/parcel_fuzzer/parcel_fuzzer.h index 765a93e8c9..dbd0caea01 100644 --- a/libs/binder/tests/parcel_fuzzer/parcel_fuzzer.h +++ b/libs/binder/tests/parcel_fuzzer/parcel_fuzzer.h @@ -15,9 +15,13 @@ */ #pragma once +#include <fuzzbinder/random_parcel.h> #include <fuzzer/FuzzedDataProvider.h> #include <functional> template <typename P> using ParcelRead = std::function<void(const P& p, FuzzedDataProvider& provider)>; +template <typename P> +using ParcelWrite = std::function<void(P& p, FuzzedDataProvider& provider, + android::RandomParcelOptions* options)>; diff --git a/libs/binder/trusty/rust/binder_rpc_test/main.rs b/libs/binder/trusty/rust/binder_rpc_test/main.rs index baea5a827b..da1a86fef4 100644 --- a/libs/binder/trusty/rust/binder_rpc_test/main.rs +++ b/libs/binder/trusty/rust/binder_rpc_test/main.rs @@ -19,7 +19,7 @@ use binder::{BinderFeatures, IBinder, Status, StatusCode, Strong}; use binder_rpc_test_aidl::aidl::IBinderRpcSession::{BnBinderRpcSession, IBinderRpcSession}; use binder_rpc_test_aidl::aidl::IBinderRpcTest::{BnBinderRpcTest, IBinderRpcTest}; use binder_rpc_test_session::MyBinderRpcSession; -use libc::{clock_gettime, CLOCK_REALTIME}; +use libc::{clock_gettime, CLOCK_BOOTTIME}; use rpcbinder::RpcSession; use trusty_std::ffi::{CString, FallibleCString}; @@ -56,7 +56,7 @@ fn get_time_ns() -> u64 { let mut ts = libc::timespec { tv_sec: 0, tv_nsec: 0 }; // Safety: Passing valid pointer to variable ts which lives past end of call - assert_eq!(unsafe { clock_gettime(CLOCK_REALTIME, &mut ts) }, 0); + assert_eq!(unsafe { clock_gettime(CLOCK_BOOTTIME, &mut ts) }, 0); ts.tv_sec as u64 * 1_000_000_000u64 + ts.tv_nsec as u64 } diff --git a/libs/binder/trusty/rust/rpcbinder/rules.mk b/libs/binder/trusty/rust/rpcbinder/rules.mk index 97f5c03cba..04c63f72f8 100644 --- a/libs/binder/trusty/rust/rpcbinder/rules.mk +++ b/libs/binder/trusty/rust/rpcbinder/rules.mk @@ -29,8 +29,8 @@ MODULE_LIBRARY_DEPS += \ $(LIBBINDER_DIR)/trusty/rust/binder_ndk_sys \ $(LIBBINDER_DIR)/trusty/rust/binder_rpc_unstable_bindgen \ $(LIBBINDER_DIR)/trusty/rust/binder_rpc_server_bindgen \ - external/rust/crates/cfg-if \ - external/rust/crates/foreign-types \ + $(call FIND_CRATE,cfg-if) \ + $(call FIND_CRATE,foreign-types) \ trusty/user/base/lib/tipc/rust \ trusty/user/base/lib/trusty-sys \ diff --git a/libs/binder/trusty/rust/rules.mk b/libs/binder/trusty/rust/rules.mk index 36bd3a2e75..e622b22246 100644 --- a/libs/binder/trusty/rust/rules.mk +++ b/libs/binder/trusty/rust/rules.mk @@ -27,8 +27,8 @@ MODULE_LIBRARY_DEPS += \ $(LIBBINDER_DIR)/trusty/ndk \ $(LIBBINDER_DIR)/trusty/rust/binder_ndk_sys \ $(LIBBINDER_DIR)/trusty/rust/binder_rpc_unstable_bindgen \ - external/rust/crates/downcast-rs \ - external/rust/crates/libc \ + $(call FIND_CRATE,downcast-rs) \ + $(call FIND_CRATE,libc) \ trusty/user/base/lib/trusty-sys \ MODULE_RUSTFLAGS += \ diff --git a/libs/ftl/non_null_test.cpp b/libs/ftl/non_null_test.cpp index 367b398915..457237c35e 100644 --- a/libs/ftl/non_null_test.cpp +++ b/libs/ftl/non_null_test.cpp @@ -81,6 +81,31 @@ static_assert(static_cast<bool>(kApplePtr)); static_assert(std::is_same_v<decltype(ftl::as_non_null(std::declval<const int* const>())), ftl::NonNull<const int*>>); +class Base {}; +class Derived : public Base {}; + +static_assert(std::is_constructible_v<ftl::NonNull<void*>, ftl::NonNull<int*>>); +static_assert(!std::is_constructible_v<ftl::NonNull<int*>, ftl::NonNull<void*>>); +static_assert(std::is_constructible_v<ftl::NonNull<const int*>, ftl::NonNull<int*>>); +static_assert(!std::is_constructible_v<ftl::NonNull<int*>, ftl::NonNull<const int*>>); +static_assert(std::is_constructible_v<ftl::NonNull<Base*>, ftl::NonNull<Derived*>>); +static_assert(!std::is_constructible_v<ftl::NonNull<Derived*>, ftl::NonNull<Base*>>); +static_assert(std::is_constructible_v<ftl::NonNull<std::unique_ptr<const int>>, + ftl::NonNull<std::unique_ptr<int>>>); +static_assert(std::is_constructible_v<ftl::NonNull<std::unique_ptr<Base>>, + ftl::NonNull<std::unique_ptr<Derived>>>); + +static_assert(std::is_assignable_v<ftl::NonNull<void*>, ftl::NonNull<int*>>); +static_assert(!std::is_assignable_v<ftl::NonNull<int*>, ftl::NonNull<void*>>); +static_assert(std::is_assignable_v<ftl::NonNull<const int*>, ftl::NonNull<int*>>); +static_assert(!std::is_assignable_v<ftl::NonNull<int*>, ftl::NonNull<const int*>>); +static_assert(std::is_assignable_v<ftl::NonNull<Base*>, ftl::NonNull<Derived*>>); +static_assert(!std::is_assignable_v<ftl::NonNull<Derived*>, ftl::NonNull<Base*>>); +static_assert(std::is_assignable_v<ftl::NonNull<std::unique_ptr<const int>>, + ftl::NonNull<std::unique_ptr<int>>>); +static_assert(std::is_assignable_v<ftl::NonNull<std::unique_ptr<Base>>, + ftl::NonNull<std::unique_ptr<Derived>>>); + } // namespace TEST(NonNull, SwapRawPtr) { @@ -156,4 +181,14 @@ TEST(NonNull, UnorderedSetOfSmartPtr) { EXPECT_TRUE(ftl::contains(spi, *spi.begin())); } +TEST(NonNull, ImplicitConversion) { + int i = 123; + int j = 345; + auto ip = ftl::as_non_null(&i); + ftl::NonNull<void*> vp{ip}; + EXPECT_EQ(vp.get(), &i); + vp = ftl::as_non_null(&j); + EXPECT_EQ(vp.get(), &j); +} + } // namespace android::test diff --git a/libs/gui/LayerState.cpp b/libs/gui/LayerState.cpp index b10996951b..422c57bc87 100644 --- a/libs/gui/LayerState.cpp +++ b/libs/gui/LayerState.cpp @@ -69,7 +69,7 @@ layer_state_t::layer_state_t() color(0), bufferTransform(0), transformToDisplayInverse(false), - crop(Rect::INVALID_RECT), + crop({0, 0, -1, -1}), dataspace(ui::Dataspace::UNKNOWN), surfaceDamageRegion(), api(-1), @@ -109,7 +109,10 @@ status_t layer_state_t::write(Parcel& output) const SAFE_PARCEL(output.writeUint32, flags); SAFE_PARCEL(output.writeUint32, mask); SAFE_PARCEL(matrix.write, output); - SAFE_PARCEL(output.write, crop); + SAFE_PARCEL(output.writeFloat, crop.top); + SAFE_PARCEL(output.writeFloat, crop.left); + SAFE_PARCEL(output.writeFloat, crop.bottom); + SAFE_PARCEL(output.writeFloat, crop.right); SAFE_PARCEL(SurfaceControl::writeNullableToParcel, output, relativeLayerSurfaceControl); SAFE_PARCEL(SurfaceControl::writeNullableToParcel, output, parentSurfaceControlForChild); SAFE_PARCEL(output.writeFloat, color.r); @@ -218,7 +221,10 @@ status_t layer_state_t::read(const Parcel& input) SAFE_PARCEL(input.readUint32, &mask); SAFE_PARCEL(matrix.read, input); - SAFE_PARCEL(input.read, crop); + SAFE_PARCEL(input.readFloat, &crop.top); + SAFE_PARCEL(input.readFloat, &crop.left); + SAFE_PARCEL(input.readFloat, &crop.bottom); + SAFE_PARCEL(input.readFloat, &crop.right); SAFE_PARCEL(SurfaceControl::readNullableFromParcel, input, &relativeLayerSurfaceControl); SAFE_PARCEL(SurfaceControl::readNullableFromParcel, input, &parentSurfaceControlForChild); diff --git a/libs/gui/SurfaceComposerClient.cpp b/libs/gui/SurfaceComposerClient.cpp index df58df43be..063aabbdef 100644 --- a/libs/gui/SurfaceComposerClient.cpp +++ b/libs/gui/SurfaceComposerClient.cpp @@ -1539,14 +1539,7 @@ SurfaceComposerClient::Transaction& SurfaceComposerClient::Transaction::setFlags mStatus = BAD_INDEX; return *this; } - if ((mask & layer_state_t::eLayerOpaque) || (mask & layer_state_t::eLayerHidden) || - (mask & layer_state_t::eLayerSecure) || (mask & layer_state_t::eLayerSkipScreenshot) || - (mask & layer_state_t::eEnableBackpressure) || - (mask & layer_state_t::eIgnoreDestinationFrame) || - (mask & layer_state_t::eLayerIsDisplayDecoration) || - (mask & layer_state_t::eLayerIsRefreshRateIndicator)) { - s->what |= layer_state_t::eFlagsChanged; - } + s->what |= layer_state_t::eFlagsChanged; s->flags &= ~mask; s->flags |= (flags & mask); s->mask |= mask; @@ -1652,6 +1645,11 @@ SurfaceComposerClient::Transaction& SurfaceComposerClient::Transaction::setMatri SurfaceComposerClient::Transaction& SurfaceComposerClient::Transaction::setCrop( const sp<SurfaceControl>& sc, const Rect& crop) { + return setCrop(sc, crop.toFloatRect()); +} + +SurfaceComposerClient::Transaction& SurfaceComposerClient::Transaction::setCrop( + const sp<SurfaceControl>& sc, const FloatRect& crop) { layer_state_t* s = getLayerState(sc); if (!s) { mStatus = BAD_INDEX; @@ -1941,6 +1939,20 @@ SurfaceComposerClient::Transaction& SurfaceComposerClient::Transaction::setDesir return *this; } +SurfaceComposerClient::Transaction& SurfaceComposerClient::Transaction::setLuts( + const sp<SurfaceControl>& sc, const base::unique_fd& /*lutFd*/, + const std::vector<int32_t>& /*offsets*/, const std::vector<int32_t>& /*dimensions*/, + const std::vector<int32_t>& /*sizes*/, const std::vector<int32_t>& /*samplingKeys*/) { + layer_state_t* s = getLayerState(sc); + if (!s) { + mStatus = BAD_INDEX; + return *this; + } + // TODO (b/329472856): update layer_state_t for lut(s) + registerSurfaceControlForCallback(sc); + return *this; +} + SurfaceComposerClient::Transaction& SurfaceComposerClient::Transaction::setCachingHint( const sp<SurfaceControl>& sc, gui::CachingHint cachingHint) { layer_state_t* s = getLayerState(sc); diff --git a/libs/gui/aidl/android/gui/Lut.aidl b/libs/gui/aidl/android/gui/Lut.aidl new file mode 100644 index 0000000000..a06e521789 --- /dev/null +++ b/libs/gui/aidl/android/gui/Lut.aidl @@ -0,0 +1,30 @@ +/* + * 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. + */ + +package android.gui; + +import android.gui.LutProperties; +import android.os.ParcelFileDescriptor; + +/** + * This mirrors aidl::android::hardware::graphics::composer3::Lut definition + * @hide + */ +parcelable Lut { + @nullable ParcelFileDescriptor pfd; + + LutProperties lutProperties; +}
\ No newline at end of file diff --git a/libs/gui/aidl/android/gui/LutProperties.aidl b/libs/gui/aidl/android/gui/LutProperties.aidl new file mode 100644 index 0000000000..561e0c069c --- /dev/null +++ b/libs/gui/aidl/android/gui/LutProperties.aidl @@ -0,0 +1,32 @@ +/* + * 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. + */ + +package android.gui; + +/** + * This mirrors aidl::android::hardware::graphics::composer3::LutProperties definition. + * @hide + */ +parcelable LutProperties { + @Backing(type="int") + enum Dimension { ONE_D = 1, THREE_D = 3 } + Dimension dimension; + + long size; + @Backing(type="int") + enum SamplingKey { RGB, MAX_RGB } + SamplingKey[] samplingKeys; +}
\ No newline at end of file diff --git a/libs/gui/aidl/android/gui/OverlayProperties.aidl b/libs/gui/aidl/android/gui/OverlayProperties.aidl index 5fb1a83c65..7f41bdab2f 100644 --- a/libs/gui/aidl/android/gui/OverlayProperties.aidl +++ b/libs/gui/aidl/android/gui/OverlayProperties.aidl @@ -16,6 +16,8 @@ package android.gui; +import android.gui.LutProperties; + /** @hide */ parcelable OverlayProperties { parcelable SupportedBufferCombinations { @@ -27,4 +29,6 @@ parcelable OverlayProperties { SupportedBufferCombinations[] combinations; boolean supportMixedColorSpaces; + + @nullable LutProperties[] lutProperties; } diff --git a/libs/gui/include/gui/LayerState.h b/libs/gui/include/gui/LayerState.h index 2cdde3255e..00065c81d8 100644 --- a/libs/gui/include/gui/LayerState.h +++ b/libs/gui/include/gui/LayerState.h @@ -330,7 +330,7 @@ struct layer_state_t { Region transparentRegion; uint32_t bufferTransform; bool transformToDisplayInverse; - Rect crop; + FloatRect crop; std::shared_ptr<BufferData> bufferData = nullptr; ui::Dataspace dataspace; HdrMetadata hdrMetadata; diff --git a/libs/gui/include/gui/SurfaceComposerClient.h b/libs/gui/include/gui/SurfaceComposerClient.h index 4f9af16826..5ea0c1619b 100644 --- a/libs/gui/include/gui/SurfaceComposerClient.h +++ b/libs/gui/include/gui/SurfaceComposerClient.h @@ -549,6 +549,7 @@ public: Transaction& setMatrix(const sp<SurfaceControl>& sc, float dsdx, float dtdx, float dtdy, float dsdy); Transaction& setCrop(const sp<SurfaceControl>& sc, const Rect& crop); + Transaction& setCrop(const sp<SurfaceControl>& sc, const FloatRect& crop); Transaction& setCornerRadius(const sp<SurfaceControl>& sc, float cornerRadius); Transaction& setBackgroundBlurRadius(const sp<SurfaceControl>& sc, int backgroundBlurRadius); @@ -601,6 +602,11 @@ public: Transaction& setExtendedRangeBrightness(const sp<SurfaceControl>& sc, float currentBufferRatio, float desiredRatio); Transaction& setDesiredHdrHeadroom(const sp<SurfaceControl>& sc, float desiredRatio); + Transaction& setLuts(const sp<SurfaceControl>& sc, const base::unique_fd& lutFd, + const std::vector<int32_t>& offsets, + const std::vector<int32_t>& dimensions, + const std::vector<int32_t>& sizes, + const std::vector<int32_t>& samplingKeys); Transaction& setCachingHint(const sp<SurfaceControl>& sc, gui::CachingHint cachingHint); Transaction& setHdrMetadata(const sp<SurfaceControl>& sc, const HdrMetadata& hdrMetadata); Transaction& setSurfaceDamageRegion(const sp<SurfaceControl>& sc, diff --git a/libs/gui/libgui_flags.aconfig b/libs/gui/libgui_flags.aconfig index d3f2899ba3..1c7e0e439c 100644 --- a/libs/gui/libgui_flags.aconfig +++ b/libs/gui/libgui_flags.aconfig @@ -109,6 +109,14 @@ flag { } # wb_libcameraservice flag { + name: "wb_unlimited_slots" + namespace: "core_graphics" + description: "Adds APIs and updates the implementation of bufferqueues to have a user-defined slot count." + bug: "341359814" + is_fixed_read_only: true +} # wb_unlimited_slots + +flag { name: "bq_producer_throttles_only_async_mode" namespace: "core_graphics" description: "BufferQueueProducer only CPU throttle on queueBuffer() in async mode." diff --git a/libs/gui/tests/SamplingDemo.cpp b/libs/gui/tests/SamplingDemo.cpp index f98437b4f8..8fea689c91 100644 --- a/libs/gui/tests/SamplingDemo.cpp +++ b/libs/gui/tests/SamplingDemo.cpp @@ -46,7 +46,8 @@ public: SurfaceComposerClient::Transaction{} .setLayer(mButton, 0x7fffffff) - .setCrop(mButton, {0, 0, width - 2 * BUTTON_PADDING, height - 2 * BUTTON_PADDING}) + .setCrop(mButton, + Rect{0, 0, width - 2 * BUTTON_PADDING, height - 2 * BUTTON_PADDING}) .setPosition(mButton, samplingArea.left + BUTTON_PADDING, samplingArea.top + BUTTON_PADDING) .setColor(mButton, half3{1, 1, 1}) @@ -59,7 +60,8 @@ public: SurfaceComposerClient::Transaction{} .setLayer(mButtonBlend, 0x7ffffffe) .setCrop(mButtonBlend, - {0, 0, width - 2 * SAMPLE_AREA_PADDING, height - 2 * SAMPLE_AREA_PADDING}) + Rect{0, 0, width - 2 * SAMPLE_AREA_PADDING, + height - 2 * SAMPLE_AREA_PADDING}) .setPosition(mButtonBlend, samplingArea.left + SAMPLE_AREA_PADDING, samplingArea.top + SAMPLE_AREA_PADDING) .setColor(mButtonBlend, half3{1, 1, 1}) @@ -75,7 +77,7 @@ public: SurfaceComposerClient::Transaction{} .setLayer(mSamplingArea, 0x7ffffffd) - .setCrop(mSamplingArea, {0, 0, 100, 32}) + .setCrop(mSamplingArea, Rect{0, 0, 100, 32}) .setPosition(mSamplingArea, 490, 1606) .setColor(mSamplingArea, half3{0, 1, 0}) .setAlpha(mSamplingArea, 0.1) diff --git a/libs/input/input_flags.aconfig b/libs/input/input_flags.aconfig index 60fb00e128..701fb43c1f 100644 --- a/libs/input/input_flags.aconfig +++ b/libs/input/input_flags.aconfig @@ -207,3 +207,10 @@ flag { description: "Allow user to enable key repeats or configure timeout before key repeat and key repeat delay rates." bug: "336585002" } + +flag { + name: "rotary_input_telemetry" + namespace: "wear_frameworks" + description: "Enable telemetry for rotary input" + bug: "370353565" +} diff --git a/libs/nativewindow/rust/Android.bp b/libs/nativewindow/rust/Android.bp index d68d6ba1af..c572ee7811 100644 --- a/libs/nativewindow/rust/Android.bp +++ b/libs/nativewindow/rust/Android.bp @@ -26,6 +26,7 @@ rust_bindgen { source_stem: "bindings", bindgen_flags: [ "--constified-enum-module=AHardwareBuffer_Format", + "--bitfield-enum=ADataSpace", "--bitfield-enum=AHardwareBuffer_UsageFlags", "--allowlist-file=.*/nativewindow/include/.*\\.h", @@ -110,6 +111,7 @@ rust_defaults { srcs: ["src/lib.rs"], rustlibs: [ "libbinder_rs", + "libbitflags", "libnativewindow_bindgen", ], } diff --git a/libs/nativewindow/rust/src/surface.rs b/libs/nativewindow/rust/src/surface.rs index 25fea807b5..9eddfcd497 100644 --- a/libs/nativewindow/rust/src/surface.rs +++ b/libs/nativewindow/rust/src/surface.rs @@ -20,10 +20,14 @@ use binder::{ unstable_api::{status_result, AsNative}, StatusCode, }; +use bitflags::bitflags; use nativewindow_bindgen::{ - AHardwareBuffer_Format, ANativeWindow, ANativeWindow_acquire, ANativeWindow_getFormat, - ANativeWindow_getHeight, ANativeWindow_getWidth, ANativeWindow_readFromParcel, - ANativeWindow_release, ANativeWindow_writeToParcel, + ADataSpace, AHardwareBuffer_Format, ANativeWindow, ANativeWindow_acquire, + ANativeWindow_getBuffersDataSpace, ANativeWindow_getBuffersDefaultDataSpace, + ANativeWindow_getFormat, ANativeWindow_getHeight, ANativeWindow_getWidth, + ANativeWindow_readFromParcel, ANativeWindow_release, ANativeWindow_setBuffersDataSpace, + ANativeWindow_setBuffersGeometry, ANativeWindow_setBuffersTransform, + ANativeWindow_writeToParcel, }; use std::error::Error; use std::fmt::{self, Debug, Display, Formatter}; @@ -60,6 +64,95 @@ impl Surface { let format = unsafe { ANativeWindow_getFormat(self.0.as_ptr()) }; format.try_into().map_err(|_| ErrorCode(format)) } + + /// Changes the format and size of the window buffers. + /// + /// The width and height control the number of pixels in the buffers, not the dimensions of the + /// window on screen. If these are different than the window's physical size, then its buffer + /// will be scaled to match that size when compositing it to the screen. The width and height + /// must be either both zero or both non-zero. If both are 0 then the window's base value will + /// come back in force. + pub fn set_buffers_geometry( + &mut self, + width: i32, + height: i32, + format: AHardwareBuffer_Format::Type, + ) -> Result<(), ErrorCode> { + // SAFETY: The ANativeWindow pointer we pass is guaranteed to be non-null and valid because + // it must have been allocated by `ANativeWindow_allocate` or `ANativeWindow_readFromParcel` + // and we have not yet released it. + let status = unsafe { + ANativeWindow_setBuffersGeometry( + self.0.as_ptr(), + width, + height, + format.try_into().expect("Invalid format"), + ) + }; + + if status == 0 { + Ok(()) + } else { + Err(ErrorCode(status)) + } + } + + /// Sets a transfom that will be applied to future buffers posted to the window. + pub fn set_buffers_transform(&mut self, transform: Transform) -> Result<(), ErrorCode> { + // SAFETY: The ANativeWindow pointer we pass is guaranteed to be non-null and valid because + // it must have been allocated by `ANativeWindow_allocate` or `ANativeWindow_readFromParcel` + // and we have not yet released it. + let status = + unsafe { ANativeWindow_setBuffersTransform(self.0.as_ptr(), transform.bits() as i32) }; + + if status == 0 { + Ok(()) + } else { + Err(ErrorCode(status)) + } + } + + /// Sets the data space that will be applied to future buffers posted to the window. + pub fn set_buffers_data_space(&mut self, data_space: ADataSpace) -> Result<(), ErrorCode> { + // SAFETY: The ANativeWindow pointer we pass is guaranteed to be non-null and valid because + // it must have been allocated by `ANativeWindow_allocate` or `ANativeWindow_readFromParcel` + // and we have not yet released it. + let status = unsafe { ANativeWindow_setBuffersDataSpace(self.0.as_ptr(), data_space.0) }; + + if status == 0 { + Ok(()) + } else { + Err(ErrorCode(status)) + } + } + + /// Gets the data space of the buffers in the window. + pub fn get_buffers_data_space(&mut self) -> Result<ADataSpace, ErrorCode> { + // SAFETY: The ANativeWindow pointer we pass is guaranteed to be non-null and valid because + // it must have been allocated by `ANativeWindow_allocate` or `ANativeWindow_readFromParcel` + // and we have not yet released it. + let data_space = unsafe { ANativeWindow_getBuffersDataSpace(self.0.as_ptr()) }; + + if data_space < 0 { + Err(ErrorCode(data_space)) + } else { + Ok(ADataSpace(data_space)) + } + } + + /// Gets the default data space of the buffers in the window as set by the consumer. + pub fn get_buffers_default_data_space(&mut self) -> Result<ADataSpace, ErrorCode> { + // SAFETY: The ANativeWindow pointer we pass is guaranteed to be non-null and valid because + // it must have been allocated by `ANativeWindow_allocate` or `ANativeWindow_readFromParcel` + // and we have not yet released it. + let data_space = unsafe { ANativeWindow_getBuffersDefaultDataSpace(self.0.as_ptr()) }; + + if data_space < 0 { + Err(ErrorCode(data_space)) + } else { + Ok(ADataSpace(data_space)) + } + } } impl Drop for Surface { @@ -141,3 +234,19 @@ impl Display for ErrorCode { write!(f, "Error {}", self.0) } } + +bitflags! { + /// Transforms that can be applied to buffers as they are displayed to a window. + #[derive(Copy, Clone, Debug, Eq, PartialEq)] + pub struct Transform: u32 { + const MIRROR_HORIZONTAL = 0x01; + const MIRROR_VERTICAL = 0x02; + const ROTATE_90 = 0x04; + } +} + +impl Transform { + pub const IDENTITY: Self = Self::empty(); + pub const ROTATE_180: Self = Self::MIRROR_HORIZONTAL.union(Self::MIRROR_VERTICAL); + pub const ROTATE_270: Self = Self::ROTATE_180.union(Self::ROTATE_90); +} diff --git a/libs/renderengine/OWNERS b/libs/renderengine/OWNERS index 66e1aa1ca1..17ab29fd29 100644 --- a/libs/renderengine/OWNERS +++ b/libs/renderengine/OWNERS @@ -5,4 +5,6 @@ alecmouri@google.com djsollen@google.com jreck@google.com lpy@google.com +nscobie@google.com +sallyqi@google.com scroggo@google.com diff --git a/libs/ui/include/ui/FloatRect.h b/libs/ui/include/ui/FloatRect.h index 4c9c7b7ef1..4366db539f 100644 --- a/libs/ui/include/ui/FloatRect.h +++ b/libs/ui/include/ui/FloatRect.h @@ -51,6 +51,9 @@ public: float bottom = 0.0f; constexpr bool isEmpty() const { return !(left < right && top < bottom); } + + // a valid rectangle has a non negative width and height + inline bool isValid() const { return (getWidth() >= 0) && (getHeight() >= 0); } }; inline bool operator==(const FloatRect& a, const FloatRect& b) { diff --git a/libs/ui/include/ui/Rect.h b/libs/ui/include/ui/Rect.h index 2eb9330cc9..2307b44eb2 100644 --- a/libs/ui/include/ui/Rect.h +++ b/libs/ui/include/ui/Rect.h @@ -74,12 +74,10 @@ public: } inline explicit Rect(const FloatRect& floatRect) { - // Ideally we would use std::round, but we don't want to add an STL - // dependency here, so we use an approximation - left = static_cast<int32_t>(floatRect.left + 0.5f); - top = static_cast<int32_t>(floatRect.top + 0.5f); - right = static_cast<int32_t>(floatRect.right + 0.5f); - bottom = static_cast<int32_t>(floatRect.bottom + 0.5f); + left = static_cast<int32_t>(std::round(floatRect.left)); + top = static_cast<int32_t>(std::round(floatRect.top)); + right = static_cast<int32_t>(std::round(floatRect.right)); + bottom = static_cast<int32_t>(std::round(floatRect.bottom)); } inline explicit Rect(const ui::Size& size) { diff --git a/libs/ui/tests/Rect_test.cpp b/libs/ui/tests/Rect_test.cpp index 9cc36bb15b..c3c8bd9fa4 100644 --- a/libs/ui/tests/Rect_test.cpp +++ b/libs/ui/tests/Rect_test.cpp @@ -99,6 +99,16 @@ TEST(RectTest, constructFromFloatRect) { EXPECT_EQ(30, rect.right); EXPECT_EQ(40, rect.bottom); } + + EXPECT_EQ(Rect(0, 1, -1, 0), Rect(FloatRect(0.f, 1.f, -1.f, 0.f))); + EXPECT_EQ(Rect(100000, 100000, -100000, -100000), + Rect(FloatRect(100000.f, 100000.f, -100000.f, -100000.f))); + + // round down if < .5 + EXPECT_EQ(Rect(0, 1, -1, 0), Rect(FloatRect(0.4f, 1.1f, -1.499f, 0.1f))); + + // round up if >= .5 + EXPECT_EQ(Rect(20, 20, -20, -20), Rect(FloatRect(19.5f, 19.9f, -19.5f, -19.9f))); } TEST(RectTest, makeInvalid) { diff --git a/libs/ultrahdr/Android.bp b/libs/ultrahdr/Android.bp deleted file mode 100644 index eda5ea4578..0000000000 --- a/libs/ultrahdr/Android.bp +++ /dev/null @@ -1,85 +0,0 @@ -// Copyright 2022 The Android Open Source Project -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. - -package { - // See: http://go/android-license-faq - default_applicable_licenses: [ - "frameworks_native_license", - "adobe_hdr_gain_map_license", - ], -} - -cc_library { - name: "libultrahdr-deprecated", - enabled: false, - host_supported: true, - vendor_available: true, - export_include_dirs: ["include"], - local_include_dirs: ["include"], - - srcs: [ - "icc.cpp", - "jpegr.cpp", - "gainmapmath.cpp", - "jpegrutils.cpp", - "multipictureformat.cpp", - ], - - shared_libs: [ - "libimage_io", - "libjpeg", - "libjpegencoder", - "libjpegdecoder", - "liblog", - "libutils", - ], -} - -cc_library { - name: "libjpegencoder-deprecated", - enabled: false, - host_supported: true, - vendor_available: true, - - shared_libs: [ - "libjpeg", - "liblog", - "libutils", - ], - - export_include_dirs: ["include"], - - srcs: [ - "jpegencoderhelper.cpp", - ], -} - -cc_library { - name: "libjpegdecoder-deprecated", - enabled: false, - host_supported: true, - vendor_available: true, - - shared_libs: [ - "libjpeg", - "liblog", - "libutils", - ], - - export_include_dirs: ["include"], - - srcs: [ - "jpegdecoderhelper.cpp", - ], -} diff --git a/libs/ultrahdr/fuzzer/Android.bp b/libs/ultrahdr/fuzzer/Android.bp deleted file mode 100644 index 8d9132fd4d..0000000000 --- a/libs/ultrahdr/fuzzer/Android.bp +++ /dev/null @@ -1,72 +0,0 @@ -// Copyright 2023 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. - -package { - // See: http://go/android-license-faq - // A large-scale-change added 'default_applicable_licenses' to import - // all of the 'license_kinds' from "frameworks_native_license" - // to get the below license kinds: - // SPDX-license-identifier-Apache-2.0 - default_applicable_licenses: ["frameworks_native_license"], -} - -cc_defaults { - name: "ultrahdr_fuzzer_defaults-deprecated", - enabled: false, - host_supported: true, - shared_libs: [ - "libimage_io", - "libjpeg", - ], - static_libs: [ - "libjpegdecoder", - "libjpegencoder", - "libultrahdr", - "libutils", - "liblog", - ], - target: { - darwin: { - enabled: false, - }, - }, - fuzz_config: { - cc: [ - "android-media-fuzzing-reports@google.com", - ], - description: "The fuzzers target the APIs of jpeg hdr", - service_privilege: "constrained", - users: "multi_user", - fuzzed_code_usage: "future_version", - vector: "local_no_privileges_required", - }, -} - -cc_fuzz { - name: "ultrahdr_enc_fuzzer-deprecated", - enabled: false, - defaults: ["ultrahdr_fuzzer_defaults"], - srcs: [ - "ultrahdr_enc_fuzzer.cpp", - ], -} - -cc_fuzz { - name: "ultrahdr_dec_fuzzer-deprecated", - enabled: false, - defaults: ["ultrahdr_fuzzer_defaults"], - srcs: [ - "ultrahdr_dec_fuzzer.cpp", - ], -} diff --git a/libs/ultrahdr/tests/Android.bp b/libs/ultrahdr/tests/Android.bp deleted file mode 100644 index 00cc797591..0000000000 --- a/libs/ultrahdr/tests/Android.bp +++ /dev/null @@ -1,51 +0,0 @@ -// Copyright 2022 The Android Open Source Project -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. - -package { - // See: http://go/android-license-faq - // A large-scale-change added 'default_applicable_licenses' to import - // all of the 'license_kinds' from "frameworks_native_license" - // to get the below license kinds: - // SPDX-license-identifier-Apache-2.0 - default_applicable_licenses: ["frameworks_native_license"], -} - -cc_test { - name: "ultrahdr_unit_test-deprecated", - enabled: false, - test_suites: ["device-tests"], - srcs: [ - "gainmapmath_test.cpp", - "icchelper_test.cpp", - "jpegr_test.cpp", - "jpegencoderhelper_test.cpp", - "jpegdecoderhelper_test.cpp", - ], - shared_libs: [ - "libimage_io", - "libjpeg", - "liblog", - ], - static_libs: [ - "libgmock", - "libgtest", - "libjpegdecoder", - "libjpegencoder", - "libultrahdr", - "libutils", - ], - data: [ - "./data/*.*", - ], -} diff --git a/opengl/libs/EGL/Loader.cpp b/opengl/libs/EGL/Loader.cpp index bf0e38e986..3be8ddca10 100644 --- a/opengl/libs/EGL/Loader.cpp +++ b/opengl/libs/EGL/Loader.cpp @@ -599,6 +599,9 @@ Loader::driver_t* Loader::attempt_to_load_angle(egl_connection_t* cnx) { driver_t* hnd = nullptr; // ANGLE doesn't ship with GLES library, and thus we skip GLES driver. + // b/370113081: if there is no libEGL_angle.so in namespace ns, libEGL_angle.so in system + // partition will be loaded instead. If there is no libEGL_angle.so in system partition, no + // angle libs are loaded, and app that sets to use ANGLE will crash. void* dso = load_angle("EGL", ns); if (dso) { initialize_api(dso, cnx, EGL); diff --git a/services/inputflinger/InputFilterCallbacks.cpp b/services/inputflinger/InputFilterCallbacks.cpp index 5fbdc84c4b..493459acfb 100644 --- a/services/inputflinger/InputFilterCallbacks.cpp +++ b/services/inputflinger/InputFilterCallbacks.cpp @@ -44,7 +44,8 @@ public: InputFilterThread(std::shared_ptr<IInputThreadCallback> callback) : mCallback(callback) { mLooper = sp<Looper>::make(/*allowNonCallbacks=*/false); mThread = std::make_unique<InputThread>( - "InputFilter", [this]() { loopOnce(); }, [this]() { mLooper->wake(); }); + "InputFilter", [this]() { loopOnce(); }, [this]() { mLooper->wake(); }, + /*isInCriticalPath=*/false); } ndk::ScopedAStatus finish() override { diff --git a/services/inputflinger/InputThread.cpp b/services/inputflinger/InputThread.cpp index 449eb45b4b..7cf4e397f9 100644 --- a/services/inputflinger/InputThread.cpp +++ b/services/inputflinger/InputThread.cpp @@ -26,6 +26,16 @@ namespace input_flags = com::android::input::flags; namespace { +bool applyInputEventProfile(const Thread& thread) { +#if defined(__ANDROID__) + return SetTaskProfiles(thread.getTid(), {"InputPolicy"}); +#else + // Since thread information is not available and there's no benefit of + // applying the task profile on host, return directly. + return true; +#endif +} + // Implementation of Thread from libutils. class InputThreadImpl : public Thread { public: @@ -45,12 +55,13 @@ private: } // namespace -InputThread::InputThread(std::string name, std::function<void()> loop, std::function<void()> wake) - : mName(name), mThreadWake(wake) { +InputThread::InputThread(std::string name, std::function<void()> loop, std::function<void()> wake, + bool isInCriticalPath) + : mThreadWake(wake) { mThread = sp<InputThreadImpl>::make(loop); - mThread->run(mName.c_str(), ANDROID_PRIORITY_URGENT_DISPLAY); - if (input_flags::enable_input_policy_profile()) { - if (!applyInputEventProfile()) { + mThread->run(name.c_str(), ANDROID_PRIORITY_URGENT_DISPLAY); + if (input_flags::enable_input_policy_profile() && isInCriticalPath) { + if (!applyInputEventProfile(*mThread)) { LOG(ERROR) << "Couldn't apply input policy profile for " << name; } } @@ -74,14 +85,4 @@ bool InputThread::isCallingThread() { #endif } -bool InputThread::applyInputEventProfile() { -#if defined(__ANDROID__) - return SetTaskProfiles(mThread->getTid(), {"InputPolicy"}); -#else - // Since thread information is not available and there's no benefit of - // applying the task profile on host, return directly. - return true; -#endif -} - -} // namespace android
\ No newline at end of file +} // namespace android diff --git a/services/inputflinger/dispatcher/InputDispatcher.cpp b/services/inputflinger/dispatcher/InputDispatcher.cpp index b0beeca43c..cbc0790f91 100644 --- a/services/inputflinger/dispatcher/InputDispatcher.cpp +++ b/services/inputflinger/dispatcher/InputDispatcher.cpp @@ -993,7 +993,8 @@ status_t InputDispatcher::start() { return ALREADY_EXISTS; } mThread = std::make_unique<InputThread>( - "InputDispatcher", [this]() { dispatchOnce(); }, [this]() { mLooper->wake(); }); + "InputDispatcher", [this]() { dispatchOnce(); }, [this]() { mLooper->wake(); }, + /*isInCriticalPath=*/true); return OK; } @@ -1029,9 +1030,8 @@ void InputDispatcher::dispatchOnce() { const nsecs_t nextAnrCheck = processAnrsLocked(); nextWakeupTime = std::min(nextWakeupTime, nextAnrCheck); - if (input_flags::enable_per_device_input_latency_metrics()) { - const nsecs_t nextStatisticsPush = processLatencyStatisticsLocked(); - nextWakeupTime = std::min(nextWakeupTime, nextStatisticsPush); + if (mPerDeviceInputLatencyMetricsFlag) { + processLatencyStatisticsLocked(); } // We are about to enter an infinitely long sleep, because we have no commands or @@ -1116,9 +1116,8 @@ nsecs_t InputDispatcher::processAnrsLocked() { /** * Check if enough time has passed since the last latency statistics push. - * Return the time at which we should wake up next. */ -nsecs_t InputDispatcher::processLatencyStatisticsLocked() { +void InputDispatcher::processLatencyStatisticsLocked() { const nsecs_t currentTime = now(); // Log the atom recording latency statistics if more than 6 hours passed from the last // push @@ -1126,7 +1125,6 @@ nsecs_t InputDispatcher::processLatencyStatisticsLocked() { mInputEventTimelineProcessor->pushLatencyStatistics(); mLastStatisticPushTime = currentTime; } - return mLastStatisticPushTime + LATENCY_STATISTICS_PUSH_INTERVAL; } std::chrono::nanoseconds InputDispatcher::getDispatchingTimeoutLocked( @@ -4548,7 +4546,7 @@ void InputDispatcher::notifyKey(const NotifyKeyArgs& args) { newEntry->traceTracker = mTracer->traceInboundEvent(*newEntry); } - if (input_flags::enable_per_device_input_latency_metrics()) { + if (mPerDeviceInputLatencyMetricsFlag) { if (args.id != android::os::IInputConstants::INVALID_INPUT_EVENT_ID && IdGenerator::getSource(args.id) == IdGenerator::Source::INPUT_READER && !mInputFilterEnabled) { diff --git a/services/inputflinger/dispatcher/InputDispatcher.h b/services/inputflinger/dispatcher/InputDispatcher.h index d90b9de80e..78cbf1eea4 100644 --- a/services/inputflinger/dispatcher/InputDispatcher.h +++ b/services/inputflinger/dispatcher/InputDispatcher.h @@ -16,6 +16,8 @@ #pragma once +#include <com_android_input_flags.h> + #include "AnrTracker.h" #include "CancelationOptions.h" #include "DragState.h" @@ -327,7 +329,7 @@ private: std::chrono::nanoseconds mMonitorDispatchingTimeout GUARDED_BY(mLock); nsecs_t processAnrsLocked() REQUIRES(mLock); - nsecs_t processLatencyStatisticsLocked() REQUIRES(mLock); + void processLatencyStatisticsLocked() REQUIRES(mLock); std::chrono::nanoseconds getDispatchingTimeoutLocked( const std::shared_ptr<Connection>& connection) REQUIRES(mLock); @@ -741,6 +743,10 @@ private: sp<android::gui::WindowInfoHandle> findWallpaperWindowBelow( const sp<android::gui::WindowInfoHandle>& windowHandle) const REQUIRES(mLock); + + /** Stores the value of the input flag for per device input latency metrics. */ + const bool mPerDeviceInputLatencyMetricsFlag = + com::android::input::flags::enable_per_device_input_latency_metrics(); }; } // namespace android::inputdispatcher diff --git a/services/inputflinger/dispatcher/trace/ThreadedBackend.cpp b/services/inputflinger/dispatcher/trace/ThreadedBackend.cpp index 3c3c15a64c..4f61885162 100644 --- a/services/inputflinger/dispatcher/trace/ThreadedBackend.cpp +++ b/services/inputflinger/dispatcher/trace/ThreadedBackend.cpp @@ -41,7 +41,7 @@ ThreadedBackend<Backend>::ThreadedBackend(Backend&& innerBackend) : mBackend(std::move(innerBackend)), mTracerThread( "InputTracer", [this]() { threadLoop(); }, - [this]() { mThreadWakeCondition.notify_all(); }) {} + [this]() { mThreadWakeCondition.notify_all(); }, /*isInCriticalPath=*/false) {} template <typename Backend> ThreadedBackend<Backend>::~ThreadedBackend() { diff --git a/services/inputflinger/include/InputThread.h b/services/inputflinger/include/InputThread.h index fcd913db7c..ed92b8f949 100644 --- a/services/inputflinger/include/InputThread.h +++ b/services/inputflinger/include/InputThread.h @@ -28,17 +28,15 @@ namespace android { */ class InputThread { public: - explicit InputThread(std::string name, std::function<void()> loop, - std::function<void()> wake = nullptr); + explicit InputThread(std::string name, std::function<void()> loop, std::function<void()> wake, + bool isInCriticalPath); virtual ~InputThread(); bool isCallingThread(); private: - std::string mName; std::function<void()> mThreadWake; sp<Thread> mThread; - bool applyInputEventProfile(); }; } // namespace android diff --git a/services/inputflinger/reader/Android.bp b/services/inputflinger/reader/Android.bp index b76e8c515f..b3cd35c936 100644 --- a/services/inputflinger/reader/Android.bp +++ b/services/inputflinger/reader/Android.bp @@ -90,10 +90,14 @@ cc_defaults { "libstatslog", "libstatspull", "libutils", + "libstatssocket", ], static_libs: [ "libchrome-gestures", "libui-types", + "libexpresslog", + "libtextclassifier_hash_static", + "libstatslog_express", ], header_libs: [ "libbatteryservice_headers", diff --git a/services/inputflinger/reader/InputReader.cpp b/services/inputflinger/reader/InputReader.cpp index e579390005..8b664d5c4e 100644 --- a/services/inputflinger/reader/InputReader.cpp +++ b/services/inputflinger/reader/InputReader.cpp @@ -122,7 +122,8 @@ status_t InputReader::start() { return ALREADY_EXISTS; } mThread = std::make_unique<InputThread>( - "InputReader", [this]() { loopOnce(); }, [this]() { mEventHub->wake(); }); + "InputReader", [this]() { loopOnce(); }, [this]() { mEventHub->wake(); }, + /*isInCriticalPath=*/true); return OK; } diff --git a/services/inputflinger/reader/mapper/RotaryEncoderInputMapper.cpp b/services/inputflinger/reader/mapper/RotaryEncoderInputMapper.cpp index b72cc6e060..c633b495e4 100644 --- a/services/inputflinger/reader/mapper/RotaryEncoderInputMapper.cpp +++ b/services/inputflinger/reader/mapper/RotaryEncoderInputMapper.cpp @@ -20,6 +20,8 @@ #include "RotaryEncoderInputMapper.h" +#include <Counter.h> +#include <com_android_input_flags.h> #include <utils/Timers.h> #include <optional> @@ -27,14 +29,26 @@ namespace android { +using android::expresslog::Counter; + +constexpr float kDefaultResolution = 0; constexpr float kDefaultScaleFactor = 1.0f; +constexpr int32_t kDefaultMinRotationsToLog = 3; RotaryEncoderInputMapper::RotaryEncoderInputMapper(InputDeviceContext& deviceContext, const InputReaderConfiguration& readerConfig) + : RotaryEncoderInputMapper(deviceContext, readerConfig, + Counter::logIncrement /* telemetryLogCounter */) {} + +RotaryEncoderInputMapper::RotaryEncoderInputMapper( + InputDeviceContext& deviceContext, const InputReaderConfiguration& readerConfig, + std::function<void(const char*, int64_t)> telemetryLogCounter) : InputMapper(deviceContext, readerConfig), mSource(AINPUT_SOURCE_ROTARY_ENCODER), mScalingFactor(kDefaultScaleFactor), - mOrientation(ui::ROTATION_0) {} + mResolution(kDefaultResolution), + mOrientation(ui::ROTATION_0), + mTelemetryLogCounter(telemetryLogCounter) {} RotaryEncoderInputMapper::~RotaryEncoderInputMapper() {} @@ -51,6 +65,7 @@ void RotaryEncoderInputMapper::populateDeviceInfo(InputDeviceInfo& info) { if (!res.has_value()) { ALOGW("Rotary Encoder device configuration file didn't specify resolution!\n"); } + mResolution = res.value_or(kDefaultResolution); std::optional<float> scalingFactor = config.getFloat("device.scalingFactor"); if (!scalingFactor.has_value()) { ALOGW("Rotary Encoder device configuration file didn't specify scaling factor," @@ -59,7 +74,22 @@ void RotaryEncoderInputMapper::populateDeviceInfo(InputDeviceInfo& info) { } mScalingFactor = scalingFactor.value_or(kDefaultScaleFactor); info.addMotionRange(AMOTION_EVENT_AXIS_SCROLL, mSource, -1.0f, 1.0f, 0.0f, 0.0f, - res.value_or(0.0f) * mScalingFactor); + mResolution * mScalingFactor); + + if (com::android::input::flags::rotary_input_telemetry()) { + mMinRotationsToLog = config.getInt("rotary_encoder.min_rotations_to_log"); + if (!mMinRotationsToLog.has_value()) { + ALOGI("Rotary Encoder device configuration file didn't specify min log rotation."); + } else if (*mMinRotationsToLog <= 0) { + ALOGE("Rotary Encoder device configuration specified non-positive min log rotation " + ": %d. Telemetry logging of rotations disabled.", + *mMinRotationsToLog); + mMinRotationsToLog = {}; + } else { + ALOGD("Rotary Encoder telemetry enabled. mMinRotationsToLog=%d", + *mMinRotationsToLog); + } + } } } @@ -121,10 +151,29 @@ std::list<NotifyArgs> RotaryEncoderInputMapper::process(const RawEvent& rawEvent return out; } +void RotaryEncoderInputMapper::logScroll(float scroll) { + if (mResolution <= 0 || !mMinRotationsToLog) return; + + mUnloggedScrolls += fabs(scroll); + + // unitsPerRotation = (2 * PI * radians) * (units per radian (i.e. resolution)) + const float unitsPerRotation = 2 * M_PI * mResolution; + const float scrollsPerMinRotationsToLog = *mMinRotationsToLog * unitsPerRotation; + const int32_t numMinRotationsToLog = + static_cast<int32_t>(mUnloggedScrolls / scrollsPerMinRotationsToLog); + mUnloggedScrolls = std::fmod(mUnloggedScrolls, scrollsPerMinRotationsToLog); + if (numMinRotationsToLog) { + mTelemetryLogCounter("input.value_rotary_input_device_full_rotation_count", + numMinRotationsToLog * (*mMinRotationsToLog)); + } +} + std::list<NotifyArgs> RotaryEncoderInputMapper::sync(nsecs_t when, nsecs_t readTime) { std::list<NotifyArgs> out; float scroll = mRotaryEncoderScrollAccumulator.getRelativeVWheel(); + logScroll(scroll); + if (mSlopController) { scroll = mSlopController->consumeEvent(when, scroll); } diff --git a/services/inputflinger/reader/mapper/RotaryEncoderInputMapper.h b/services/inputflinger/reader/mapper/RotaryEncoderInputMapper.h index 7e804150b1..d74ced180e 100644 --- a/services/inputflinger/reader/mapper/RotaryEncoderInputMapper.h +++ b/services/inputflinger/reader/mapper/RotaryEncoderInputMapper.h @@ -46,13 +46,39 @@ private: int32_t mSource; float mScalingFactor; + /** Units per rotation, provided via the `device.res` IDC property. */ + float mResolution; ui::Rotation mOrientation; + /** + * The minimum number of rotations to log for telemetry. + * Provided via `rotary_encoder.min_rotations_to_log` IDC property. If no value is provided in + * the IDC file, or if a non-positive value is provided, the telemetry will be disabled, and + * this value is set to the empty optional. + */ + std::optional<int32_t> mMinRotationsToLog; + /** + * A function to log count for telemetry. + * The char* is the logging key, and the int64_t is the value to log. + * Abstracting the actual logging APIs via this function is helpful for simple unit testing. + */ + std::function<void(const char*, int64_t)> mTelemetryLogCounter; ui::LogicalDisplayId mDisplayId = ui::LogicalDisplayId::INVALID; std::unique_ptr<SlopController> mSlopController; + /** Amount of raw scrolls (pre-slop) not yet logged for telemetry. */ + float mUnloggedScrolls = 0; + explicit RotaryEncoderInputMapper(InputDeviceContext& deviceContext, const InputReaderConfiguration& readerConfig); + + /** This is a test constructor that allows injecting the expresslog Counter logic. */ + RotaryEncoderInputMapper(InputDeviceContext& deviceContext, + const InputReaderConfiguration& readerConfig, + std::function<void(const char*, int64_t)> expressLogCounter); [[nodiscard]] std::list<NotifyArgs> sync(nsecs_t when, nsecs_t readTime); + + /** Logs a given amount of scroll for telemetry. */ + void logScroll(float scroll); }; } // namespace android diff --git a/services/inputflinger/rust/bounce_keys_filter.rs b/services/inputflinger/rust/bounce_keys_filter.rs index e05e8e512f..a8959d9041 100644 --- a/services/inputflinger/rust/bounce_keys_filter.rs +++ b/services/inputflinger/rust/bounce_keys_filter.rs @@ -25,6 +25,7 @@ use com_android_server_inputflinger::aidl::com::android::server::inputflinger::{ }; use input::KeyboardType; use log::debug; +use std::any::Any; use std::collections::{HashMap, HashSet}; #[derive(Debug)] @@ -134,6 +135,17 @@ impl Filter for BounceKeysFilter { self.next.destroy(); } + fn save( + &mut self, + state: HashMap<&'static str, Box<dyn Any + Send + Sync>>, + ) -> HashMap<&'static str, Box<dyn Any + Send + Sync>> { + self.next.save(state) + } + + fn restore(&mut self, state: &HashMap<&'static str, Box<dyn Any + Send + Sync>>) { + self.next.restore(state); + } + fn dump(&mut self, dump_str: String) -> String { let mut result = "Bounce Keys filter: \n".to_string(); result += &format!("\tthreshold = {:?}ns\n", self.bounce_key_threshold_ns); diff --git a/services/inputflinger/rust/input_filter.rs b/services/inputflinger/rust/input_filter.rs index e2212449aa..39c3465aea 100644 --- a/services/inputflinger/rust/input_filter.rs +++ b/services/inputflinger/rust/input_filter.rs @@ -33,6 +33,8 @@ use crate::slow_keys_filter::SlowKeysFilter; use crate::sticky_keys_filter::StickyKeysFilter; use input::ModifierState; use log::{error, info}; +use std::any::Any; +use std::collections::HashMap; use std::sync::{Arc, Mutex, RwLock}; /// Virtual keyboard device ID @@ -43,6 +45,11 @@ pub trait Filter { fn notify_key(&mut self, event: &KeyEvent); fn notify_devices_changed(&mut self, device_infos: &[DeviceInfo]); fn destroy(&mut self); + fn save( + &mut self, + state: HashMap<&'static str, Box<dyn Any + Send + Sync>>, + ) -> HashMap<&'static str, Box<dyn Any + Send + Sync>>; + fn restore(&mut self, state: &HashMap<&'static str, Box<dyn Any + Send + Sync>>); fn dump(&mut self, dump_str: String) -> String; } @@ -105,6 +112,7 @@ impl IInputFilter for InputFilter { fn notifyConfigurationChanged(&self, config: &InputFilterConfiguration) -> binder::Result<()> { { let mut state = self.state.lock().unwrap(); + let saved_state = state.first_filter.save(HashMap::new()); state.first_filter.destroy(); let mut first_filter: Box<dyn Filter + Send + Sync> = Box::new(BaseFilter::new(self.callbacks.clone())); @@ -138,6 +146,7 @@ impl IInputFilter for InputFilter { ); } state.first_filter = first_filter; + state.first_filter.restore(&saved_state); } Result::Ok(()) } @@ -175,6 +184,18 @@ impl Filter for BaseFilter { // do nothing } + fn save( + &mut self, + state: HashMap<&'static str, Box<dyn Any + Send + Sync>>, + ) -> HashMap<&'static str, Box<dyn Any + Send + Sync>> { + // do nothing + state + } + + fn restore(&mut self, _state: &HashMap<&'static str, Box<dyn Any + Send + Sync>>) { + // do nothing + } + fn dump(&mut self, dump_str: String) -> String { // do nothing dump_str @@ -367,6 +388,8 @@ pub mod test_filter { use com_android_server_inputflinger::aidl::com::android::server::inputflinger::{ DeviceInfo::DeviceInfo, KeyEvent::KeyEvent, }; + use std::any::Any; + use std::collections::HashMap; use std::sync::{Arc, RwLock, RwLockWriteGuard}; #[derive(Default)] @@ -415,6 +438,16 @@ pub mod test_filter { fn destroy(&mut self) { self.inner().is_destroy_called = true; } + fn save( + &mut self, + state: HashMap<&'static str, Box<dyn Any + Send + Sync>>, + ) -> HashMap<&'static str, Box<dyn Any + Send + Sync>> { + // do nothing + state + } + fn restore(&mut self, _state: &HashMap<&'static str, Box<dyn Any + Send + Sync>>) { + // do nothing + } fn dump(&mut self, dump_str: String) -> String { // do nothing dump_str diff --git a/services/inputflinger/rust/slow_keys_filter.rs b/services/inputflinger/rust/slow_keys_filter.rs index 8830aac529..085e80e207 100644 --- a/services/inputflinger/rust/slow_keys_filter.rs +++ b/services/inputflinger/rust/slow_keys_filter.rs @@ -26,7 +26,8 @@ use com_android_server_inputflinger::aidl::com::android::server::inputflinger::{ }; use input::KeyboardType; use log::debug; -use std::collections::HashSet; +use std::any::Any; +use std::collections::{HashMap, HashSet}; use std::sync::{Arc, RwLock, RwLockReadGuard, RwLockWriteGuard}; // Policy flags from Input.h @@ -187,6 +188,19 @@ impl Filter for SlowKeysFilter { slow_filter.next.destroy(); } + fn save( + &mut self, + state: HashMap<&'static str, Box<dyn Any + Send + Sync>>, + ) -> HashMap<&'static str, Box<dyn Any + Send + Sync>> { + let mut slow_filter = self.write_inner(); + slow_filter.next.save(state) + } + + fn restore(&mut self, state: &HashMap<&'static str, Box<dyn Any + Send + Sync>>) { + let mut slow_filter = self.write_inner(); + slow_filter.next.restore(state); + } + fn dump(&mut self, dump_str: String) -> String { let mut slow_filter = self.write_inner(); let mut result = "Slow Keys filter: \n".to_string(); diff --git a/services/inputflinger/rust/sticky_keys_filter.rs b/services/inputflinger/rust/sticky_keys_filter.rs index 161a5fca60..7a1d0ec934 100644 --- a/services/inputflinger/rust/sticky_keys_filter.rs +++ b/services/inputflinger/rust/sticky_keys_filter.rs @@ -24,7 +24,8 @@ use com_android_server_inputflinger::aidl::com::android::server::inputflinger::{ DeviceInfo::DeviceInfo, KeyEvent::KeyEvent, KeyEventAction::KeyEventAction, }; use input::ModifierState; -use std::collections::HashSet; +use std::any::Any; +use std::collections::{HashMap, HashSet}; // Modifier keycodes: values are from /frameworks/native/include/android/keycodes.h const KEYCODE_ALT_LEFT: i32 = 57; @@ -40,10 +41,17 @@ const KEYCODE_META_LEFT: i32 = 117; const KEYCODE_META_RIGHT: i32 = 118; const KEYCODE_FUNCTION: i32 = 119; const KEYCODE_NUM_LOCK: i32 = 143; +static STICKY_KEYS_DATA: &str = "sticky_keys_data"; pub struct StickyKeysFilter { next: Box<dyn Filter + Send + Sync>, listener: ModifierStateListener, + data: Data, +} + +#[derive(Default)] +/// Data that will be saved and restored across configuration changes +struct Data { /// Tracking devices that contributed to the modifier state. contributing_devices: HashSet<i32>, /// State describing the current enabled modifiers. This contain both locked and non-locked @@ -61,21 +69,15 @@ impl StickyKeysFilter { next: Box<dyn Filter + Send + Sync>, listener: ModifierStateListener, ) -> StickyKeysFilter { - Self { - next, - listener, - contributing_devices: HashSet::new(), - modifier_state: ModifierState::None, - locked_modifier_state: ModifierState::None, - } + Self { next, listener, data: Default::default() } } } impl Filter for StickyKeysFilter { fn notify_key(&mut self, event: &KeyEvent) { let up = event.action == KeyEventAction::UP; - let mut modifier_state = self.modifier_state; - let mut locked_modifier_state = self.locked_modifier_state; + let mut modifier_state = self.data.modifier_state; + let mut locked_modifier_state = self.data.locked_modifier_state; if !is_ephemeral_modifier_key(event.keyCode) { // If non-ephemeral modifier key (i.e. non-modifier keys + toggle modifier keys like // CAPS_LOCK, NUM_LOCK etc.), don't block key and pass in the sticky modifier state with @@ -93,7 +95,7 @@ impl Filter for StickyKeysFilter { } } else if up { // Update contributing devices to track keyboards - self.contributing_devices.insert(event.deviceId); + self.data.contributing_devices.insert(event.deviceId); // If ephemeral modifier key, capture the key and update the sticky modifier states let modifier_key_mask = get_ephemeral_modifier_key_mask(event.keyCode); let symmetrical_modifier_key_mask = get_symmetrical_modifier_key_mask(event.keyCode); @@ -108,38 +110,62 @@ impl Filter for StickyKeysFilter { modifier_state |= modifier_key_mask; } } - if self.modifier_state != modifier_state - || self.locked_modifier_state != locked_modifier_state + if self.data.modifier_state != modifier_state + || self.data.locked_modifier_state != locked_modifier_state { - self.modifier_state = modifier_state; - self.locked_modifier_state = locked_modifier_state; + self.data.modifier_state = modifier_state; + self.data.locked_modifier_state = locked_modifier_state; self.listener.modifier_state_changed(modifier_state, locked_modifier_state); } } fn notify_devices_changed(&mut self, device_infos: &[DeviceInfo]) { // Clear state if all contributing devices removed - self.contributing_devices.retain(|id| device_infos.iter().any(|x| *id == x.deviceId)); - if self.contributing_devices.is_empty() - && (self.modifier_state != ModifierState::None - || self.locked_modifier_state != ModifierState::None) + self.data.contributing_devices.retain(|id| device_infos.iter().any(|x| *id == x.deviceId)); + if self.data.contributing_devices.is_empty() + && (self.data.modifier_state != ModifierState::None + || self.data.locked_modifier_state != ModifierState::None) { - self.modifier_state = ModifierState::None; - self.locked_modifier_state = ModifierState::None; + self.data.modifier_state = ModifierState::None; + self.data.locked_modifier_state = ModifierState::None; self.listener.modifier_state_changed(ModifierState::None, ModifierState::None); } self.next.notify_devices_changed(device_infos); } + fn save( + &mut self, + mut state: HashMap<&'static str, Box<dyn Any + Send + Sync>>, + ) -> HashMap<&'static str, Box<dyn Any + Send + Sync>> { + let data = Data { + contributing_devices: self.data.contributing_devices.clone(), + modifier_state: self.data.modifier_state, + locked_modifier_state: self.data.locked_modifier_state, + }; + state.insert(STICKY_KEYS_DATA, Box::new(data)); + self.next.save(state) + } + + fn restore(&mut self, state: &HashMap<&'static str, Box<dyn Any + Send + Sync>>) { + if let Some(value) = state.get(STICKY_KEYS_DATA) { + if let Some(data) = value.downcast_ref::<Data>() { + self.data.contributing_devices = data.contributing_devices.clone(); + self.data.modifier_state = data.modifier_state; + self.data.locked_modifier_state = data.locked_modifier_state; + } + } + self.next.restore(state) + } + fn destroy(&mut self) { self.next.destroy(); } fn dump(&mut self, dump_str: String) -> String { let mut result = "Sticky Keys filter: \n".to_string(); - result += &format!("\tmodifier_state = {:?}\n", self.modifier_state); - result += &format!("\tlocked_modifier_state = {:?}\n", self.locked_modifier_state); - result += &format!("\tcontributing_devices = {:?}\n", self.contributing_devices); + result += &format!("\tmodifier_state = {:?}\n", self.data.modifier_state); + result += &format!("\tlocked_modifier_state = {:?}\n", self.data.locked_modifier_state); + result += &format!("\tcontributing_devices = {:?}\n", self.data.contributing_devices); self.next.dump(dump_str + &result) } } @@ -245,6 +271,7 @@ mod tests { }; use input::KeyboardType; use input::ModifierState; + use std::collections::HashMap; use std::sync::{Arc, RwLock}; static DEVICE_ID: i32 = 1; @@ -452,6 +479,45 @@ mod tests { } #[test] + fn test_modifier_state_restored_on_recreation() { + let test_filter = TestFilter::new(); + let test_callbacks = TestCallbacks::new(); + let mut sticky_keys_filter = setup_filter( + Box::new(test_filter.clone()), + Arc::new(RwLock::new(Strong::new(Box::new(test_callbacks.clone())))), + ); + sticky_keys_filter.notify_key(&KeyEvent { keyCode: KEYCODE_CTRL_LEFT, ..BASE_KEY_DOWN }); + sticky_keys_filter.notify_key(&KeyEvent { keyCode: KEYCODE_CTRL_LEFT, ..BASE_KEY_UP }); + + let saved_state = sticky_keys_filter.save(HashMap::new()); + sticky_keys_filter.destroy(); + + // Create a new Sticky keys filter + let test_filter = TestFilter::new(); + let mut sticky_keys_filter = setup_filter( + Box::new(test_filter.clone()), + Arc::new(RwLock::new(Strong::new(Box::new(test_callbacks.clone())))), + ); + sticky_keys_filter.restore(&saved_state); + assert_eq!( + test_callbacks.get_last_modifier_state(), + ModifierState::CtrlLeftOn | ModifierState::CtrlOn + ); + assert_eq!(test_callbacks.get_last_locked_modifier_state(), ModifierState::None); + + sticky_keys_filter.notify_key(&KeyEvent { keyCode: KEYCODE_CTRL_LEFT, ..BASE_KEY_DOWN }); + sticky_keys_filter.notify_key(&KeyEvent { keyCode: KEYCODE_CTRL_LEFT, ..BASE_KEY_UP }); + assert_eq!( + test_callbacks.get_last_modifier_state(), + ModifierState::CtrlLeftOn | ModifierState::CtrlOn + ); + assert_eq!( + test_callbacks.get_last_locked_modifier_state(), + ModifierState::CtrlLeftOn | ModifierState::CtrlOn + ); + } + + #[test] fn test_key_events_have_sticky_modifier_state() { let test_filter = TestFilter::new(); let test_callbacks = TestCallbacks::new(); diff --git a/services/inputflinger/tests/RotaryEncoderInputMapper_test.cpp b/services/inputflinger/tests/RotaryEncoderInputMapper_test.cpp index 6607bc7972..157bee33e1 100644 --- a/services/inputflinger/tests/RotaryEncoderInputMapper_test.cpp +++ b/services/inputflinger/tests/RotaryEncoderInputMapper_test.cpp @@ -23,6 +23,8 @@ #include <android-base/logging.h> #include <android_companion_virtualdevice_flags.h> +#include <com_android_input_flags.h> +#include <flag_macros.h> #include <gtest/gtest.h> #include <input/DisplayViewport.h> #include <linux/input-event-codes.h> @@ -100,6 +102,15 @@ protected: EXPECT_CALL(mMockEventHub, hasRelativeAxis(EVENTHUB_ID, REL_HWHEEL_HI_RES)) .WillRepeatedly(Return(false)); } + + std::map<std::string, int64_t> mTelemetryLogCounts; + + /** + * A fake function for telemetry logging. + * Records the log counts in the `mTelemetryLogCounts` map. + */ + std::function<void(const char*, int64_t)> mTelemetryLogCounter = + [this](const char* key, int64_t value) { mTelemetryLogCounts[key] += value; }; }; TEST_F(RotaryEncoderInputMapperTest, ConfigureDisplayIdWithAssociatedViewport) { @@ -187,4 +198,142 @@ TEST_F(RotaryEncoderInputMapperTest, HighResScrollIgnoresRegularScroll) { WithMotionAction(AMOTION_EVENT_ACTION_SCROLL), WithScroll(0.5f))))); } +TEST_F_WITH_FLAGS(RotaryEncoderInputMapperTest, RotaryInputTelemetryFlagOff_NoRotationLogging, + REQUIRES_FLAGS_DISABLED(ACONFIG_FLAG(com::android::input::flags, + rotary_input_telemetry))) { + mPropertyMap.addProperty("device.res", "3"); + mMapper = createInputMapper<RotaryEncoderInputMapper>(*mDeviceContext, mReaderConfiguration, + mTelemetryLogCounter); + InputDeviceInfo info; + mMapper->populateDeviceInfo(info); + + std::list<NotifyArgs> args; + args += process(ARBITRARY_TIME, EV_REL, REL_WHEEL, 70); + args += process(ARBITRARY_TIME, EV_SYN, SYN_REPORT, 0); + + ASSERT_EQ(mTelemetryLogCounts.find("input.value_rotary_input_device_full_rotation_count"), + mTelemetryLogCounts.end()); +} + +TEST_F_WITH_FLAGS(RotaryEncoderInputMapperTest, ZeroResolution_NoRotationLogging, + REQUIRES_FLAGS_ENABLED(ACONFIG_FLAG(com::android::input::flags, + rotary_input_telemetry))) { + mPropertyMap.addProperty("device.res", "-3"); + mPropertyMap.addProperty("rotary_encoder.min_rotations_to_log", "2"); + mMapper = createInputMapper<RotaryEncoderInputMapper>(*mDeviceContext, mReaderConfiguration, + mTelemetryLogCounter); + InputDeviceInfo info; + mMapper->populateDeviceInfo(info); + + std::list<NotifyArgs> args; + args += process(ARBITRARY_TIME, EV_REL, REL_WHEEL, 700); + args += process(ARBITRARY_TIME, EV_SYN, SYN_REPORT, 0); + + ASSERT_EQ(mTelemetryLogCounts.find("input.value_rotary_input_device_full_rotation_count"), + mTelemetryLogCounts.end()); +} + +TEST_F_WITH_FLAGS(RotaryEncoderInputMapperTest, NegativeMinLogRotation_NoRotationLogging, + REQUIRES_FLAGS_ENABLED(ACONFIG_FLAG(com::android::input::flags, + rotary_input_telemetry))) { + mPropertyMap.addProperty("device.res", "3"); + mPropertyMap.addProperty("rotary_encoder.min_rotations_to_log", "-2"); + mMapper = createInputMapper<RotaryEncoderInputMapper>(*mDeviceContext, mReaderConfiguration, + mTelemetryLogCounter); + InputDeviceInfo info; + mMapper->populateDeviceInfo(info); + + std::list<NotifyArgs> args; + args += process(ARBITRARY_TIME, EV_REL, REL_WHEEL, 700); + args += process(ARBITRARY_TIME, EV_SYN, SYN_REPORT, 0); + + ASSERT_EQ(mTelemetryLogCounts.find("input.value_rotary_input_device_full_rotation_count"), + mTelemetryLogCounts.end()); +} + +TEST_F_WITH_FLAGS(RotaryEncoderInputMapperTest, ZeroMinLogRotation_NoRotationLogging, + REQUIRES_FLAGS_ENABLED(ACONFIG_FLAG(com::android::input::flags, + rotary_input_telemetry))) { + mPropertyMap.addProperty("device.res", "3"); + mPropertyMap.addProperty("rotary_encoder.min_rotations_to_log", "0"); + mMapper = createInputMapper<RotaryEncoderInputMapper>(*mDeviceContext, mReaderConfiguration, + mTelemetryLogCounter); + InputDeviceInfo info; + mMapper->populateDeviceInfo(info); + + std::list<NotifyArgs> args; + args += process(ARBITRARY_TIME, EV_REL, REL_WHEEL, 700); + args += process(ARBITRARY_TIME, EV_SYN, SYN_REPORT, 0); + + ASSERT_EQ(mTelemetryLogCounts.find("input.value_rotary_input_device_full_rotation_count"), + mTelemetryLogCounts.end()); +} + +TEST_F_WITH_FLAGS(RotaryEncoderInputMapperTest, NoMinLogRotation_NoRotationLogging, + REQUIRES_FLAGS_ENABLED(ACONFIG_FLAG(com::android::input::flags, + rotary_input_telemetry))) { + // 3 units per radian, 2 * M_PI * 3 = ~18.85 units per rotation. + mPropertyMap.addProperty("device.res", "3"); + mMapper = createInputMapper<RotaryEncoderInputMapper>(*mDeviceContext, mReaderConfiguration, + mTelemetryLogCounter); + InputDeviceInfo info; + mMapper->populateDeviceInfo(info); + + std::list<NotifyArgs> args; + args += process(ARBITRARY_TIME, EV_REL, REL_WHEEL, 700); + args += process(ARBITRARY_TIME, EV_SYN, SYN_REPORT, 0); + + ASSERT_EQ(mTelemetryLogCounts.find("input.value_rotary_input_device_full_rotation_count"), + mTelemetryLogCounts.end()); +} + +TEST_F_WITH_FLAGS(RotaryEncoderInputMapperTest, RotationLogging, + REQUIRES_FLAGS_ENABLED(ACONFIG_FLAG(com::android::input::flags, + rotary_input_telemetry))) { + // 3 units per radian, 2 * M_PI * 3 = ~18.85 units per rotation. + // Multiples of `unitsPerRoation`, to easily follow the assertions below. + // [18.85, 37.7, 56.55, 75.4, 94.25, 113.1, 131.95, 150.8] + mPropertyMap.addProperty("device.res", "3"); + mPropertyMap.addProperty("rotary_encoder.min_rotations_to_log", "2"); + + mMapper = createInputMapper<RotaryEncoderInputMapper>(*mDeviceContext, mReaderConfiguration, + mTelemetryLogCounter); + InputDeviceInfo info; + mMapper->populateDeviceInfo(info); + + std::list<NotifyArgs> args; + args += process(ARBITRARY_TIME, EV_REL, REL_WHEEL, 15); // total scroll = 15 + args += process(ARBITRARY_TIME, EV_SYN, SYN_REPORT, 0); + ASSERT_EQ(mTelemetryLogCounts.find("input.value_rotary_input_device_full_rotation_count"), + mTelemetryLogCounts.end()); + + args += process(ARBITRARY_TIME, EV_REL, REL_WHEEL, 13); // total scroll = 28 + args += process(ARBITRARY_TIME, EV_SYN, SYN_REPORT, 0); + // Expect 0 since `min_rotations_to_log` = 2, and total scroll 28 only has 1 rotation. + ASSERT_EQ(mTelemetryLogCounts.find("input.value_rotary_input_device_full_rotation_count"), + mTelemetryLogCounts.end()); + + args += process(ARBITRARY_TIME, EV_REL, REL_WHEEL, 10); // total scroll = 38 + args += process(ARBITRARY_TIME, EV_SYN, SYN_REPORT, 0); + // Total scroll includes >= `min_rotations_to_log` (2), expect log. + ASSERT_EQ(mTelemetryLogCounts["input.value_rotary_input_device_full_rotation_count"], 2); + + args += process(ARBITRARY_TIME, EV_REL, REL_WHEEL, -22); // total scroll = 60 + args += process(ARBITRARY_TIME, EV_SYN, SYN_REPORT, 0); + // Expect no additional telemetry. Total rotation is 3, and total unlogged rotation is 1, which + // is less than `min_rotations_to_log`. + ASSERT_EQ(mTelemetryLogCounts["input.value_rotary_input_device_full_rotation_count"], 2); + + args += process(ARBITRARY_TIME, EV_REL, REL_WHEEL, -16); // total scroll = 76 + args += process(ARBITRARY_TIME, EV_SYN, SYN_REPORT, 0); + // Total unlogged rotation >= `min_rotations_to_log` (2), so expect 2 more logged rotation. + ASSERT_EQ(mTelemetryLogCounts["input.value_rotary_input_device_full_rotation_count"], 4); + + args += process(ARBITRARY_TIME, EV_REL, REL_WHEEL, -76); // total scroll = 152 + args += process(ARBITRARY_TIME, EV_SYN, SYN_REPORT, 0); + // Total unlogged scroll >= 4*`min_rotations_to_log`. Expect *all* unlogged rotations to be + // logged, even if that's more than multiple of `min_rotations_to_log`. + ASSERT_EQ(mTelemetryLogCounts["input.value_rotary_input_device_full_rotation_count"], 8); +} + } // namespace android
\ No newline at end of file diff --git a/services/surfaceflinger/Android.bp b/services/surfaceflinger/Android.bp index c2a9880d87..8161b4731e 100644 --- a/services/surfaceflinger/Android.bp +++ b/services/surfaceflinger/Android.bp @@ -173,7 +173,6 @@ filegroup { "DisplayHardware/VirtualDisplaySurface.cpp", "DisplayRenderArea.cpp", "Effects/Daltonizer.cpp", - "EventLog/EventLog.cpp", "FrontEnd/LayerCreationArgs.cpp", "FrontEnd/LayerHandle.cpp", "FrontEnd/LayerSnapshot.cpp", diff --git a/services/surfaceflinger/CompositionEngine/include/compositionengine/LayerFECompositionState.h b/services/surfaceflinger/CompositionEngine/include/compositionengine/LayerFECompositionState.h index d1429a2ec6..14a8fd6ad7 100644 --- a/services/surfaceflinger/CompositionEngine/include/compositionengine/LayerFECompositionState.h +++ b/services/surfaceflinger/CompositionEngine/include/compositionengine/LayerFECompositionState.h @@ -155,7 +155,7 @@ struct LayerFECompositionState { uint32_t geomBufferTransform{0}; Rect geomBufferSize; Rect geomContentCrop; - Rect geomCrop; + FloatRect geomCrop; GenericLayerMetadataMap metadata; diff --git a/services/surfaceflinger/CompositionEngine/include/compositionengine/OutputLayer.h b/services/surfaceflinger/CompositionEngine/include/compositionengine/OutputLayer.h index 4dbf8d2fce..dcfe21a849 100644 --- a/services/surfaceflinger/CompositionEngine/include/compositionengine/OutputLayer.h +++ b/services/surfaceflinger/CompositionEngine/include/compositionengine/OutputLayer.h @@ -128,6 +128,10 @@ public: // Applies a HWC device layer request virtual void applyDeviceLayerRequest(Hwc2::IComposerClient::LayerRequest request) = 0; + // Applies a HWC device layer lut + virtual void applyDeviceLayerLut(aidl::android::hardware::graphics::composer3::LutProperties, + ndk::ScopedFileDescriptor) = 0; + // Returns true if the composition settings scale pixels virtual bool needsFiltering() const = 0; diff --git a/services/surfaceflinger/CompositionEngine/include/compositionengine/impl/Display.h b/services/surfaceflinger/CompositionEngine/include/compositionengine/impl/Display.h index d1eff241d3..a39abb40d2 100644 --- a/services/surfaceflinger/CompositionEngine/include/compositionengine/impl/Display.h +++ b/services/surfaceflinger/CompositionEngine/include/compositionengine/impl/Display.h @@ -82,11 +82,13 @@ public: using DisplayRequests = android::HWComposer::DeviceRequestedChanges::DisplayRequests; using LayerRequests = android::HWComposer::DeviceRequestedChanges::LayerRequests; using ClientTargetProperty = android::HWComposer::DeviceRequestedChanges::ClientTargetProperty; + using LayerLuts = android::HWComposer::DeviceRequestedChanges::LayerLuts; virtual bool allLayersRequireClientComposition() const; virtual void applyChangedTypesToLayers(const ChangedTypes&); virtual void applyDisplayRequests(const DisplayRequests&); virtual void applyLayerRequestsToLayers(const LayerRequests&); virtual void applyClientTargetRequests(const ClientTargetProperty&); + virtual void applyLayerLutsToLayers(const LayerLuts&); // Internal virtual void setConfiguration(const compositionengine::DisplayCreationArgs&); diff --git a/services/surfaceflinger/CompositionEngine/include/compositionengine/impl/OutputLayer.h b/services/surfaceflinger/CompositionEngine/include/compositionengine/impl/OutputLayer.h index f383392e55..354a4416f2 100644 --- a/services/surfaceflinger/CompositionEngine/include/compositionengine/impl/OutputLayer.h +++ b/services/surfaceflinger/CompositionEngine/include/compositionengine/impl/OutputLayer.h @@ -60,6 +60,8 @@ public: aidl::android::hardware::graphics::composer3::Composition) override; void prepareForDeviceLayerRequests() override; void applyDeviceLayerRequest(Hwc2::IComposerClient::LayerRequest request) override; + void applyDeviceLayerLut(aidl::android::hardware::graphics::composer3::LutProperties, + ndk::ScopedFileDescriptor) override; bool needsFiltering() const override; std::optional<LayerFE::LayerSettings> getOverrideCompositionSettings() const override; diff --git a/services/surfaceflinger/CompositionEngine/include/compositionengine/mock/OutputLayer.h b/services/surfaceflinger/CompositionEngine/include/compositionengine/mock/OutputLayer.h index 5fef63a510..48c2f9c483 100644 --- a/services/surfaceflinger/CompositionEngine/include/compositionengine/mock/OutputLayer.h +++ b/services/surfaceflinger/CompositionEngine/include/compositionengine/mock/OutputLayer.h @@ -56,6 +56,9 @@ public: MOCK_METHOD1(applyDeviceLayerRequest, void(Hwc2::IComposerClient::LayerRequest request)); MOCK_CONST_METHOD0(needsFiltering, bool()); MOCK_CONST_METHOD0(getOverrideCompositionSettings, std::optional<LayerFE::LayerSettings>()); + MOCK_METHOD(void, applyDeviceLayerLut, + (aidl::android::hardware::graphics::composer3::LutProperties, + ndk::ScopedFileDescriptor)); MOCK_CONST_METHOD1(dump, void(std::string&)); }; diff --git a/services/surfaceflinger/CompositionEngine/src/Display.cpp b/services/surfaceflinger/CompositionEngine/src/Display.cpp index 77b1940e23..b0164b7c33 100644 --- a/services/surfaceflinger/CompositionEngine/src/Display.cpp +++ b/services/surfaceflinger/CompositionEngine/src/Display.cpp @@ -278,6 +278,7 @@ void Display::applyCompositionStrategy(const std::optional<DeviceRequestedChange applyDisplayRequests(changes->displayRequests); applyLayerRequestsToLayers(changes->layerRequests); applyClientTargetRequests(changes->clientTargetProperty); + applyLayerLutsToLayers(changes->layerLuts); } // Determine what type of composition we are doing from the final state @@ -359,6 +360,25 @@ void Display::applyClientTargetRequests(const ClientTargetProperty& clientTarget static_cast<ui::PixelFormat>(clientTargetProperty.clientTargetProperty.pixelFormat)); } +void Display::applyLayerLutsToLayers(const LayerLuts& layerLuts) { + auto& mapper = getCompositionEngine().getHwComposer().getLutFileDescriptorMapper(); + for (auto* layer : getOutputLayersOrderedByZ()) { + auto hwcLayer = layer->getHwcLayer(); + if (!hwcLayer) { + continue; + } + + if (auto lutsIt = layerLuts.find(hwcLayer); lutsIt != layerLuts.end()) { + if (auto mapperIt = mapper.find(hwcLayer); mapperIt != mapper.end()) { + layer->applyDeviceLayerLut(lutsIt->second, + ndk::ScopedFileDescriptor(mapperIt->second.release())); + } + } + } + + mapper.clear(); +} + void Display::executeCommands() { const auto halDisplayIdOpt = HalDisplayId::tryCast(mId); if (mIsDisconnected || !halDisplayIdOpt) { diff --git a/services/surfaceflinger/CompositionEngine/src/OutputLayer.cpp b/services/surfaceflinger/CompositionEngine/src/OutputLayer.cpp index 091c207464..2d46dc0702 100644 --- a/services/surfaceflinger/CompositionEngine/src/OutputLayer.cpp +++ b/services/surfaceflinger/CompositionEngine/src/OutputLayer.cpp @@ -24,6 +24,7 @@ #include <compositionengine/impl/OutputLayerCompositionState.h> #include <cstdint> #include "system/graphics-base-v1.0.h" +#include "ui/FloatRect.h" #include <ui/HdrRenderTypeUtils.h> @@ -37,6 +38,7 @@ #pragma clang diagnostic pop // ignored "-Wconversion" using aidl::android::hardware::graphics::composer3::Composition; +using aidl::android::hardware::graphics::composer3::LutProperties; namespace android::compositionengine { @@ -185,35 +187,35 @@ Rect OutputLayer::calculateOutputDisplayFrame() const { const auto& layerState = *getLayerFE().getCompositionState(); const auto& outputState = getOutput().getState(); + // Convert from layer space to layerStackSpace // apply the layer's transform, followed by the display's global transform // here we're guaranteed that the layer's transform preserves rects - Region activeTransparentRegion = layerState.transparentRegionHint; const ui::Transform& layerTransform = layerState.geomLayerTransform; - const ui::Transform& inverseLayerTransform = layerState.geomInverseLayerTransform; - const Rect& bufferSize = layerState.geomBufferSize; - Rect activeCrop = layerState.geomCrop; - if (!activeCrop.isEmpty() && bufferSize.isValid()) { - activeCrop = layerTransform.transform(activeCrop); - if (!activeCrop.intersect(outputState.layerStackSpace.getContent(), &activeCrop)) { - activeCrop.clear(); - } - activeCrop = inverseLayerTransform.transform(activeCrop, true); - // This needs to be here as transform.transform(Rect) computes the - // transformed rect and then takes the bounding box of the result before - // returning. This means - // transform.inverse().transform(transform.transform(Rect)) != Rect - // in which case we need to make sure the final rect is clipped to the - // display bounds. - if (!activeCrop.intersect(bufferSize, &activeCrop)) { - activeCrop.clear(); - } + Region activeTransparentRegion = layerTransform.transform(layerState.transparentRegionHint); + if (!layerState.geomCrop.isEmpty() && layerState.geomBufferSize.isValid()) { + FloatRect activeCrop = layerTransform.transform(layerState.geomCrop); + activeCrop = activeCrop.intersect(outputState.layerStackSpace.getContent().toFloatRect()); + const FloatRect& bufferSize = + layerTransform.transform(layerState.geomBufferSize.toFloatRect()); + activeCrop = activeCrop.intersect(bufferSize); + // mark regions outside the crop as transparent - activeTransparentRegion.orSelf(Rect(0, 0, bufferSize.getWidth(), activeCrop.top)); - activeTransparentRegion.orSelf( - Rect(0, activeCrop.bottom, bufferSize.getWidth(), bufferSize.getHeight())); - activeTransparentRegion.orSelf(Rect(0, activeCrop.top, activeCrop.left, activeCrop.bottom)); - activeTransparentRegion.orSelf( - Rect(activeCrop.right, activeCrop.top, bufferSize.getWidth(), activeCrop.bottom)); + Rect topRegion = Rect(layerTransform.transform( + FloatRect(0, 0, layerState.geomBufferSize.getWidth(), layerState.geomCrop.top))); + Rect bottomRegion = Rect(layerTransform.transform( + FloatRect(0, layerState.geomCrop.bottom, layerState.geomBufferSize.getWidth(), + layerState.geomBufferSize.getHeight()))); + Rect leftRegion = Rect(layerTransform.transform(FloatRect(0, layerState.geomCrop.top, + layerState.geomCrop.left, + layerState.geomCrop.bottom))); + Rect rightRegion = Rect(layerTransform.transform( + FloatRect(layerState.geomCrop.right, layerState.geomCrop.top, + layerState.geomBufferSize.getWidth(), layerState.geomCrop.bottom))); + + activeTransparentRegion.orSelf(topRegion); + activeTransparentRegion.orSelf(bottomRegion); + activeTransparentRegion.orSelf(leftRegion); + activeTransparentRegion.orSelf(rightRegion); } // reduce uses a FloatRect to provide more accuracy during the @@ -223,19 +225,22 @@ Rect OutputLayer::calculateOutputDisplayFrame() const { // Some HWCs may clip client composited input to its displayFrame. Make sure // that this does not cut off the shadow. if (layerState.forceClientComposition && layerState.shadowSettings.length > 0.0f) { - const auto outset = layerState.shadowSettings.length; + // RenderEngine currently blurs shadows to smooth out edges, so outset by + // 2x the length instead of 1x to compensate + const auto outset = layerState.shadowSettings.length * 2; geomLayerBounds.left -= outset; geomLayerBounds.top -= outset; geomLayerBounds.right += outset; geomLayerBounds.bottom += outset; } - Rect frame{layerTransform.transform(reduce(geomLayerBounds, activeTransparentRegion))}; - if (!frame.intersect(outputState.layerStackSpace.getContent(), &frame)) { - frame.clear(); - } - const ui::Transform displayTransform{outputState.transform}; - return displayTransform.transform(frame); + geomLayerBounds = layerTransform.transform(geomLayerBounds); + FloatRect frame = reduce(geomLayerBounds, activeTransparentRegion); + frame = frame.intersect(outputState.layerStackSpace.getContent().toFloatRect()); + + // convert from layerStackSpace to displaySpace + const ui::Transform displayTransform{outputState.transform}; + return Rect(displayTransform.transform(frame)); } uint32_t OutputLayer::calculateOutputRelativeBufferTransform( @@ -844,6 +849,12 @@ void OutputLayer::applyDeviceLayerRequest(hal::LayerRequest request) { } } +void OutputLayer::applyDeviceLayerLut(LutProperties /*lutProperties*/, + ndk::ScopedFileDescriptor /*lutPfd*/) { + // TODO(b/329472856): decode the shared memory of the pfd, and store the lut data into + // OutputLayerCompositionState#hwc struct +} + bool OutputLayer::needsFiltering() const { const auto& state = getState(); const auto& sourceCrop = state.sourceCrop; diff --git a/services/surfaceflinger/CompositionEngine/tests/DisplayTest.cpp b/services/surfaceflinger/CompositionEngine/tests/DisplayTest.cpp index 39163ea60f..9c0e62c643 100644 --- a/services/surfaceflinger/CompositionEngine/tests/DisplayTest.cpp +++ b/services/surfaceflinger/CompositionEngine/tests/DisplayTest.cpp @@ -133,6 +133,7 @@ struct DisplayTestCommon : public testing::Test { MOCK_METHOD1(applyChangedTypesToLayers, void(const impl::Display::ChangedTypes&)); MOCK_METHOD1(applyDisplayRequests, void(const impl::Display::DisplayRequests&)); MOCK_METHOD1(applyLayerRequestsToLayers, void(const impl::Display::LayerRequests&)); + MOCK_METHOD1(applyLayerLutsToLayers, void(const impl::Display::LayerLuts&)); const compositionengine::CompositionEngine& mCompositionEngine; impl::OutputCompositionState mState; @@ -212,6 +213,7 @@ struct PartialMockDisplayTestCommon : public DisplayTestCommon { aidl::android::hardware::graphics::common::Dataspace::UNKNOWN}, -1.f, DimmingStage::NONE}, + {}, }; void chooseCompositionStrategy(Display* display) { @@ -615,6 +617,7 @@ TEST_F(DisplayChooseCompositionStrategyTest, normalOperation) { EXPECT_CALL(*mDisplay, applyLayerRequestsToLayers(mDeviceRequestedChanges.layerRequests)) .Times(1); EXPECT_CALL(*mDisplay, allLayersRequireClientComposition()).WillOnce(Return(false)); + EXPECT_CALL(*mDisplay, applyLayerLutsToLayers(mDeviceRequestedChanges.layerLuts)).Times(1); chooseCompositionStrategy(mDisplay.get()); @@ -667,6 +670,7 @@ TEST_F(DisplayChooseCompositionStrategyTest, normalOperationWithChanges) { EXPECT_CALL(*mDisplay, applyLayerRequestsToLayers(mDeviceRequestedChanges.layerRequests)) .Times(1); EXPECT_CALL(*mDisplay, allLayersRequireClientComposition()).WillOnce(Return(false)); + EXPECT_CALL(*mDisplay, applyLayerLutsToLayers(mDeviceRequestedChanges.layerLuts)).Times(1); chooseCompositionStrategy(mDisplay.get()); diff --git a/services/surfaceflinger/CompositionEngine/tests/MockHWComposer.h b/services/surfaceflinger/CompositionEngine/tests/MockHWComposer.h index e910c72e2e..5c55ce739a 100644 --- a/services/surfaceflinger/CompositionEngine/tests/MockHWComposer.h +++ b/services/surfaceflinger/CompositionEngine/tests/MockHWComposer.h @@ -152,10 +152,7 @@ public: getOverlaySupport, (), (const, override)); MOCK_METHOD(status_t, setRefreshRateChangedCallbackDebugEnabled, (PhysicalDisplayId, bool)); MOCK_METHOD(status_t, notifyExpectedPresent, (PhysicalDisplayId, TimePoint, Fps)); - MOCK_METHOD(status_t, getRequestedLuts, - (PhysicalDisplayId, - std::vector<aidl::android::hardware::graphics::composer3::DisplayLuts::LayerLut>*), - (override)); + MOCK_METHOD((HWC2::Display::LutFileDescriptorMapper&), getLutFileDescriptorMapper, (), ()); }; } // namespace mock diff --git a/services/surfaceflinger/CompositionEngine/tests/OutputLayerTest.cpp b/services/surfaceflinger/CompositionEngine/tests/OutputLayerTest.cpp index 1c54469cc0..b21533a009 100644 --- a/services/surfaceflinger/CompositionEngine/tests/OutputLayerTest.cpp +++ b/services/surfaceflinger/CompositionEngine/tests/OutputLayerTest.cpp @@ -30,6 +30,7 @@ #include "MockHWC2.h" #include "MockHWComposer.h" #include "RegionMatcher.h" +#include "ui/FloatRect.h" #include <aidl/android/hardware/graphics/composer3/Composition.h> @@ -270,7 +271,7 @@ struct OutputLayerDisplayFrameTest : public OutputLayerTest { mLayerFEState.geomLayerTransform = ui::Transform{TR_IDENT}; mLayerFEState.geomBufferSize = Rect{0, 0, 1920, 1080}; mLayerFEState.geomBufferUsesDisplayInverseTransform = false; - mLayerFEState.geomCrop = Rect{0, 0, 1920, 1080}; + mLayerFEState.geomCrop = FloatRect{0, 0, 1920, 1080}; mLayerFEState.geomLayerBounds = FloatRect{0.f, 0.f, 1920.f, 1080.f}; mOutputState.layerStackSpace.setContent(Rect{0, 0, 1920, 1080}); @@ -296,20 +297,20 @@ TEST_F(OutputLayerDisplayFrameTest, fullActiveTransparentRegionReturnsEmptyFrame } TEST_F(OutputLayerDisplayFrameTest, cropAffectsDisplayFrame) { - mLayerFEState.geomCrop = Rect{100, 200, 300, 500}; + mLayerFEState.geomCrop = FloatRect{100, 200, 300, 500}; const Rect expected{100, 200, 300, 500}; EXPECT_THAT(calculateOutputDisplayFrame(), expected); } TEST_F(OutputLayerDisplayFrameTest, cropAffectsDisplayFrameRotated) { - mLayerFEState.geomCrop = Rect{100, 200, 300, 500}; + mLayerFEState.geomCrop = FloatRect{100, 200, 300, 500}; mLayerFEState.geomLayerTransform.set(HAL_TRANSFORM_ROT_90, 1920, 1080); const Rect expected{1420, 100, 1720, 300}; EXPECT_THAT(calculateOutputDisplayFrame(), expected); } TEST_F(OutputLayerDisplayFrameTest, emptyGeomCropIsNotUsedToComputeFrame) { - mLayerFEState.geomCrop = Rect{}; + mLayerFEState.geomCrop = FloatRect{}; const Rect expected{0, 0, 1920, 1080}; EXPECT_THAT(calculateOutputDisplayFrame(), expected); } @@ -339,7 +340,7 @@ TEST_F(OutputLayerDisplayFrameTest, shadowExpandsDisplayFrame) { mLayerFEState.geomLayerBounds = FloatRect{100.f, 100.f, 200.f, 200.f}; Rect expected{mLayerFEState.geomLayerBounds}; - expected.inset(-kShadowRadius, -kShadowRadius, -kShadowRadius, -kShadowRadius); + expected.inset(-2 * kShadowRadius, -2 * kShadowRadius, -2 * kShadowRadius, -2 * kShadowRadius); EXPECT_THAT(calculateOutputDisplayFrame(), expected); } diff --git a/services/surfaceflinger/Display/DisplayModeController.cpp b/services/surfaceflinger/Display/DisplayModeController.cpp index 0e9218cb93..f8b6c6e5d5 100644 --- a/services/surfaceflinger/Display/DisplayModeController.cpp +++ b/services/surfaceflinger/Display/DisplayModeController.cpp @@ -283,6 +283,8 @@ void DisplayModeController::updateKernelIdleTimer(PhysicalDisplayId displayId, } } +#pragma clang diagnostic push +#pragma clang diagnostic ignored "-Wunused-value" // b/369277774 auto DisplayModeController::getKernelIdleTimerState(PhysicalDisplayId displayId) const -> KernelIdleTimerState { std::lock_guard lock(mDisplayLock); @@ -298,4 +300,5 @@ auto DisplayModeController::getKernelIdleTimerState(PhysicalDisplayId displayId) return {desiredModeIdOpt, displayPtr->isKernelIdleTimerEnabled}; } +#pragma clang diagnostic pop } // namespace android::display diff --git a/services/surfaceflinger/DisplayHardware/AidlComposerHal.cpp b/services/surfaceflinger/DisplayHardware/AidlComposerHal.cpp index 66237b9d8a..77bd8040c5 100644 --- a/services/surfaceflinger/DisplayHardware/AidlComposerHal.cpp +++ b/services/surfaceflinger/DisplayHardware/AidlComposerHal.cpp @@ -1547,7 +1547,8 @@ Error AidlComposer::getClientTargetProperty( return error; } -Error AidlComposer::getRequestedLuts(Display display, std::vector<DisplayLuts::LayerLut>* outLuts) { +Error AidlComposer::getRequestedLuts(Display display, std::vector<Layer>* outLayers, + std::vector<DisplayLuts::LayerLut>* outLuts) { Error error = Error::NONE; mMutex.lock_shared(); if (auto reader = getReader(display)) { @@ -1556,6 +1557,11 @@ Error AidlComposer::getRequestedLuts(Display display, std::vector<DisplayLuts::L error = Error::BAD_DISPLAY; } mMutex.unlock_shared(); + + outLayers->reserve(outLuts->size()); + for (const auto& layerLut : *outLuts) { + outLayers->emplace_back(translate<Layer>(layerLut.layer)); + } return error; } diff --git a/services/surfaceflinger/DisplayHardware/AidlComposerHal.h b/services/surfaceflinger/DisplayHardware/AidlComposerHal.h index 246223a668..cdb67e4e5c 100644 --- a/services/surfaceflinger/DisplayHardware/AidlComposerHal.h +++ b/services/surfaceflinger/DisplayHardware/AidlComposerHal.h @@ -245,7 +245,7 @@ public: Error notifyExpectedPresent(Display, nsecs_t expectedPresentTime, int32_t frameIntervalNs) override; Error getRequestedLuts( - Display display, + Display display, std::vector<Layer>* outLayers, std::vector<aidl::android::hardware::graphics::composer3::DisplayLuts::LayerLut>* outLuts) override; Error setLayerLuts( diff --git a/services/surfaceflinger/DisplayHardware/ComposerHal.h b/services/surfaceflinger/DisplayHardware/ComposerHal.h index 7db9a94e54..09056631d0 100644 --- a/services/surfaceflinger/DisplayHardware/ComposerHal.h +++ b/services/surfaceflinger/DisplayHardware/ComposerHal.h @@ -305,7 +305,7 @@ public: virtual Error setRefreshRateChangedCallbackDebugEnabled(Display, bool) = 0; virtual Error notifyExpectedPresent(Display, nsecs_t expectedPresentTime, int32_t frameIntervalNs) = 0; - virtual Error getRequestedLuts(Display display, + virtual Error getRequestedLuts(Display display, std::vector<Layer>* outLayers, std::vector<V3_0::DisplayLuts::LayerLut>* outLuts) = 0; virtual Error setLayerLuts(Display display, Layer layer, std::vector<V3_0::Lut>& luts) = 0; }; diff --git a/services/surfaceflinger/DisplayHardware/HWC2.cpp b/services/surfaceflinger/DisplayHardware/HWC2.cpp index f1fa9389eb..1df2ab12ce 100644 --- a/services/surfaceflinger/DisplayHardware/HWC2.cpp +++ b/services/surfaceflinger/DisplayHardware/HWC2.cpp @@ -609,17 +609,29 @@ Error Display::getClientTargetProperty( return static_cast<Error>(error); } -Error Display::getRequestedLuts(std::vector<DisplayLuts::LayerLut>* outLayerLuts) { - std::vector<DisplayLuts::LayerLut> tmpLayerLuts; - const auto error = mComposer.getRequestedLuts(mId, &tmpLayerLuts); - for (DisplayLuts::LayerLut& layerLut : tmpLayerLuts) { - if (layerLut.lut.pfd.get() >= 0) { - outLayerLuts->push_back({layerLut.layer, - Lut{ndk::ScopedFileDescriptor(layerLut.lut.pfd.release()), - layerLut.lut.lutProperties}}); +Error Display::getRequestedLuts(LayerLuts* outLuts, + LutFileDescriptorMapper& lutFileDescriptorMapper) { + std::vector<Hwc2::Layer> layerIds; + std::vector<DisplayLuts::LayerLut> tmpLuts; + const auto error = static_cast<Error>(mComposer.getRequestedLuts(mId, &layerIds, &tmpLuts)); + if (error != Error::NONE) { + return error; + } + + uint32_t numElements = layerIds.size(); + outLuts->clear(); + for (uint32_t i = 0; i < numElements; ++i) { + auto layer = getLayerById(layerIds[i]); + if (layer) { + auto& layerLut = tmpLuts[i]; + outLuts->emplace_or_replace(layer.get(), layerLut.lut.lutProperties); + lutFileDescriptorMapper.emplace_or_replace(layer.get(), + ndk::ScopedFileDescriptor( + layerLut.lut.pfd.release())); } } - return static_cast<Error>(error); + + return Error::NONE; } Error Display::getDisplayDecorationSupport( diff --git a/services/surfaceflinger/DisplayHardware/HWC2.h b/services/surfaceflinger/DisplayHardware/HWC2.h index 8e2aeaf234..61f92f4d74 100644 --- a/services/surfaceflinger/DisplayHardware/HWC2.h +++ b/services/surfaceflinger/DisplayHardware/HWC2.h @@ -20,6 +20,7 @@ #include <android-base/thread_annotations.h> #include <ftl/expected.h> #include <ftl/future.h> +#include <ftl/small_map.h> #include <gui/HdrMetadata.h> #include <math/mat4.h> #include <ui/HdrCapabilities.h> @@ -107,6 +108,13 @@ public: virtual void onLayerDestroyed(hal::HWLayerId layerId) = 0; virtual std::optional<ui::Size> getPhysicalSizeInMm() const = 0; + static const int kLutFileDescriptorMapperSize = 20; + using LayerLuts = + ftl::SmallMap<HWC2::Layer*, aidl::android::hardware::graphics::composer3::LutProperties, + kLutFileDescriptorMapperSize>; + using LutFileDescriptorMapper = + ftl::SmallMap<HWC2::Layer*, ndk::ScopedFileDescriptor, kLutFileDescriptorMapperSize>; + [[nodiscard]] virtual hal::Error acceptChanges() = 0; [[nodiscard]] virtual base::expected<std::shared_ptr<HWC2::Layer>, hal::Error> createLayer() = 0; @@ -183,8 +191,7 @@ public: aidl::android::hardware::graphics::composer3::ClientTargetPropertyWithBrightness* outClientTargetProperty) = 0; [[nodiscard]] virtual hal::Error getRequestedLuts( - std::vector<aidl::android::hardware::graphics::composer3::DisplayLuts::LayerLut>* - outLuts) = 0; + LayerLuts* outLuts, LutFileDescriptorMapper& lutFileDescriptorMapper) = 0; [[nodiscard]] virtual hal::Error getDisplayDecorationSupport( std::optional<aidl::android::hardware::graphics::common::DisplayDecorationSupport>* support) = 0; @@ -268,9 +275,8 @@ public: hal::Error getClientTargetProperty( aidl::android::hardware::graphics::composer3::ClientTargetPropertyWithBrightness* outClientTargetProperty) override; - hal::Error getRequestedLuts( - std::vector<aidl::android::hardware::graphics::composer3::DisplayLuts::LayerLut>* - outLuts) override; + hal::Error getRequestedLuts(LayerLuts* outLuts, + LutFileDescriptorMapper& lutFileDescriptorMapper) override; hal::Error getDisplayDecorationSupport( std::optional<aidl::android::hardware::graphics::common::DisplayDecorationSupport>* support) override; diff --git a/services/surfaceflinger/DisplayHardware/HWComposer.cpp b/services/surfaceflinger/DisplayHardware/HWComposer.cpp index d08e261e6c..e37c0ba296 100644 --- a/services/surfaceflinger/DisplayHardware/HWComposer.cpp +++ b/services/surfaceflinger/DisplayHardware/HWComposer.cpp @@ -587,9 +587,14 @@ status_t HWComposer::getDeviceCompositionChanges( error = hwcDisplay->getClientTargetProperty(&clientTargetProperty); RETURN_IF_HWC_ERROR_FOR("getClientTargetProperty", error, displayId, BAD_INDEX); + DeviceRequestedChanges::LayerLuts layerLuts; + error = hwcDisplay->getRequestedLuts(&layerLuts, mLutFileDescriptorMapper); + RETURN_IF_HWC_ERROR_FOR("getRequestedLuts", error, displayId, BAD_INDEX); + outChanges->emplace(DeviceRequestedChanges{std::move(changedTypes), std::move(displayRequests), std::move(layerRequests), - std::move(clientTargetProperty)}); + std::move(clientTargetProperty), + std::move(layerLuts)}); error = hwcDisplay->acceptChanges(); RETURN_IF_HWC_ERROR_FOR("acceptChanges", error, displayId, BAD_INDEX); @@ -978,21 +983,6 @@ status_t HWComposer::getDisplayDecorationSupport( return NO_ERROR; } -status_t HWComposer::getRequestedLuts( - PhysicalDisplayId displayId, - std::vector<aidl::android::hardware::graphics::composer3::DisplayLuts::LayerLut>* outLuts) { - RETURN_IF_INVALID_DISPLAY(displayId, BAD_INDEX); - const auto error = mDisplayData[displayId].hwcDisplay->getRequestedLuts(outLuts); - if (error == hal::Error::UNSUPPORTED) { - RETURN_IF_HWC_ERROR(error, displayId, INVALID_OPERATION); - } - if (error == hal::Error::BAD_PARAMETER) { - RETURN_IF_HWC_ERROR(error, displayId, BAD_VALUE); - } - RETURN_IF_HWC_ERROR(error, displayId, UNKNOWN_ERROR); - return NO_ERROR; -} - status_t HWComposer::setAutoLowLatencyMode(PhysicalDisplayId displayId, bool on) { RETURN_IF_INVALID_DISPLAY(displayId, BAD_INDEX); const auto error = mDisplayData[displayId].hwcDisplay->setAutoLowLatencyMode(on); @@ -1036,6 +1026,11 @@ const std::unordered_map<std::string, bool>& HWComposer::getSupportedLayerGeneri return mSupportedLayerGenericMetadata; } +ftl::SmallMap<HWC2::Layer*, ndk::ScopedFileDescriptor, 20>& +HWComposer::getLutFileDescriptorMapper() { + return mLutFileDescriptorMapper; +} + void HWComposer::dumpOverlayProperties(std::string& result) const { // dump overlay properties result.append("OverlayProperties:\n"); diff --git a/services/surfaceflinger/DisplayHardware/HWComposer.h b/services/surfaceflinger/DisplayHardware/HWComposer.h index b95c619f75..7b04d6755a 100644 --- a/services/surfaceflinger/DisplayHardware/HWComposer.h +++ b/services/surfaceflinger/DisplayHardware/HWComposer.h @@ -53,6 +53,7 @@ #include <aidl/android/hardware/graphics/composer3/Composition.h> #include <aidl/android/hardware/graphics/composer3/DisplayCapability.h> #include <aidl/android/hardware/graphics/composer3/DisplayLuts.h> +#include <aidl/android/hardware/graphics/composer3/LutProperties.h> #include <aidl/android/hardware/graphics/composer3/OverlayProperties.h> namespace android { @@ -90,11 +91,14 @@ public: aidl::android::hardware::graphics::composer3::ClientTargetPropertyWithBrightness; using DisplayRequests = hal::DisplayRequest; using LayerRequests = std::unordered_map<HWC2::Layer*, hal::LayerRequest>; + using LutProperties = aidl::android::hardware::graphics::composer3::LutProperties; + using LayerLuts = HWC2::Display::LayerLuts; ChangedTypes changedTypes; DisplayRequests displayRequests; LayerRequests layerRequests; ClientTargetProperty clientTargetProperty; + LayerLuts layerLuts; }; struct HWCDisplayMode { @@ -311,18 +315,15 @@ public: virtual status_t setRefreshRateChangedCallbackDebugEnabled(PhysicalDisplayId, bool enabled) = 0; virtual status_t notifyExpectedPresent(PhysicalDisplayId, TimePoint expectedPresentTime, Fps frameInterval) = 0; - - // Composer 4.0 - virtual status_t getRequestedLuts( - PhysicalDisplayId, - std::vector<aidl::android::hardware::graphics::composer3::DisplayLuts::LayerLut>*) = 0; + // mapper + virtual HWC2::Display::LutFileDescriptorMapper& getLutFileDescriptorMapper() = 0; }; static inline bool operator==(const android::HWComposer::DeviceRequestedChanges& lhs, const android::HWComposer::DeviceRequestedChanges& rhs) { return lhs.changedTypes == rhs.changedTypes && lhs.displayRequests == rhs.displayRequests && lhs.layerRequests == rhs.layerRequests && - lhs.clientTargetProperty == rhs.clientTargetProperty; + lhs.clientTargetProperty == rhs.clientTargetProperty && lhs.layerLuts == rhs.layerLuts; } namespace impl { @@ -480,11 +481,8 @@ public: status_t notifyExpectedPresent(PhysicalDisplayId, TimePoint expectedPresentTime, Fps frameInterval) override; - // Composer 4.0 - status_t getRequestedLuts( - PhysicalDisplayId, - std::vector<aidl::android::hardware::graphics::composer3::DisplayLuts::LayerLut>*) - override; + // get a mapper + HWC2::Display::LutFileDescriptorMapper& getLutFileDescriptorMapper() override; // for debugging ---------------------------------------------------------- void dump(std::string& out) const override; @@ -571,6 +569,8 @@ private: const size_t mMaxVirtualDisplayDimension; const bool mUpdateDeviceProductInfoOnHotplugReconnect; bool mEnableVrrTimeout; + + HWC2::Display::LutFileDescriptorMapper mLutFileDescriptorMapper; }; } // namespace impl diff --git a/services/surfaceflinger/DisplayHardware/HidlComposerHal.cpp b/services/surfaceflinger/DisplayHardware/HidlComposerHal.cpp index ee1e07ae7d..056ecd78f4 100644 --- a/services/surfaceflinger/DisplayHardware/HidlComposerHal.cpp +++ b/services/surfaceflinger/DisplayHardware/HidlComposerHal.cpp @@ -1410,7 +1410,8 @@ Error HidlComposer::getClientTargetProperty( return Error::NONE; } -Error HidlComposer::getRequestedLuts(Display, std::vector<DisplayLuts::LayerLut>*) { +Error HidlComposer::getRequestedLuts(Display, std::vector<Layer>*, + std::vector<DisplayLuts::LayerLut>*) { return Error::NONE; } diff --git a/services/surfaceflinger/DisplayHardware/HidlComposerHal.h b/services/surfaceflinger/DisplayHardware/HidlComposerHal.h index 701a54bc66..1cc23d1409 100644 --- a/services/surfaceflinger/DisplayHardware/HidlComposerHal.h +++ b/services/surfaceflinger/DisplayHardware/HidlComposerHal.h @@ -352,7 +352,7 @@ public: Error setRefreshRateChangedCallbackDebugEnabled(Display, bool) override; Error notifyExpectedPresent(Display, nsecs_t, int32_t) override; Error getRequestedLuts( - Display, + Display, std::vector<Layer>*, std::vector<aidl::android::hardware::graphics::composer3::DisplayLuts::LayerLut>*) override; Error setLayerLuts(Display, Layer, diff --git a/services/surfaceflinger/EventLog/EventLog.cpp b/services/surfaceflinger/EventLog/EventLog.cpp deleted file mode 100644 index 3b609524e8..0000000000 --- a/services/surfaceflinger/EventLog/EventLog.cpp +++ /dev/null @@ -1,133 +0,0 @@ -/* - * Copyright 2013 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. - */ - -// TODO(b/129481165): remove the #pragma below and fix conversion issues -#pragma clang diagnostic push -#pragma clang diagnostic ignored "-Wconversion" - -#include <stdio.h> -#include <stdlib.h> -#include <log/log.h> - -#include "EventLog.h" - -namespace android { - -ANDROID_SINGLETON_STATIC_INSTANCE(EventLog) - - -EventLog::EventLog() { -} - -void EventLog::doLogFrameDurations(const std::string_view& name, const int32_t* durations, - size_t numDurations) { - EventLog::TagBuffer buffer(LOGTAG_SF_FRAME_DUR); - buffer.startList(1 + numDurations); - buffer.writeString(name); - for (size_t i = 0; i < numDurations; i++) { - buffer.writeInt32(durations[i]); - } - buffer.endList(); - buffer.log(); -} - -void EventLog::logFrameDurations(const std::string_view& name, const int32_t* durations, - size_t numDurations) { - EventLog::getInstance().doLogFrameDurations(name, durations, numDurations); -} - -// --------------------------------------------------------------------------- - -EventLog::TagBuffer::TagBuffer(int32_t tag) - : mPos(0), mTag(tag), mOverflow(false) { -} - -void EventLog::TagBuffer::log() { - if (mOverflow) { - ALOGW("couldn't log to binary event log: overflow."); - } else if (android_bWriteLog(mTag, mStorage, mPos) < 0) { - ALOGE("couldn't log to EventLog: %s", strerror(errno)); - } - // purge the buffer - mPos = 0; - mOverflow = false; -} - -void EventLog::TagBuffer::startList(int8_t count) { - if (mOverflow) return; - const size_t needed = 1 + sizeof(count); - if (mPos + needed > STORAGE_MAX_SIZE) { - mOverflow = true; - return; - } - mStorage[mPos + 0] = EVENT_TYPE_LIST; - mStorage[mPos + 1] = count; - mPos += needed; -} - -void EventLog::TagBuffer::endList() { - if (mOverflow) return; - const size_t needed = 1; - if (mPos + needed > STORAGE_MAX_SIZE) { - mOverflow = true; - return; - } - mStorage[mPos + 0] = '\n'; - mPos += needed; -} - -void EventLog::TagBuffer::writeInt32(int32_t value) { - if (mOverflow) return; - const size_t needed = 1 + sizeof(value); - if (mPos + needed > STORAGE_MAX_SIZE) { - mOverflow = true; - return; - } - mStorage[mPos + 0] = EVENT_TYPE_INT; - memcpy(&mStorage[mPos + 1], &value, sizeof(value)); - mPos += needed; -} - -void EventLog::TagBuffer::writeInt64(int64_t value) { - if (mOverflow) return; - const size_t needed = 1 + sizeof(value); - if (mPos + needed > STORAGE_MAX_SIZE) { - mOverflow = true; - return; - } - mStorage[mPos + 0] = EVENT_TYPE_LONG; - memcpy(&mStorage[mPos + 1], &value, sizeof(value)); - mPos += needed; -} - -void EventLog::TagBuffer::writeString(const std::string_view& value) { - if (mOverflow) return; - const size_t stringLen = value.length(); - const size_t needed = 1 + sizeof(int32_t) + stringLen; - if (mPos + needed > STORAGE_MAX_SIZE) { - mOverflow = true; - return; - } - mStorage[mPos + 0] = EVENT_TYPE_STRING; - memcpy(&mStorage[mPos + 1], &stringLen, sizeof(int32_t)); - memcpy(&mStorage[mPos + 5], value.data(), stringLen); - mPos += needed; -} - -} // namespace android - -// TODO(b/129481165): remove the #pragma below and fix conversion issues -#pragma clang diagnostic pop // ignored "-Wconversion" diff --git a/services/surfaceflinger/EventLog/EventLog.h b/services/surfaceflinger/EventLog/EventLog.h deleted file mode 100644 index ee3587ef8a..0000000000 --- a/services/surfaceflinger/EventLog/EventLog.h +++ /dev/null @@ -1,73 +0,0 @@ -/* - * Copyright 2013 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 <utils/Errors.h> -#include <utils/Singleton.h> - -#include <cstdint> -#include <string_view> - -namespace android { - -class EventLog : public Singleton<EventLog> { - -public: - static void logFrameDurations(const std::string_view& name, const int32_t* durations, - size_t numDurations); - -protected: - EventLog(); - -private: - /* - * EventLogBuffer is a helper class to construct an in-memory event log - * tag. In this version the buffer is not dynamic, so write operation can - * fail if there is not enough space in the temporary buffer. - * Once constructed, the buffer can be logger by calling the log() - * method. - */ - - class TagBuffer { - enum { STORAGE_MAX_SIZE = 128 }; - int32_t mPos; - int32_t mTag; - bool mOverflow; - char mStorage[STORAGE_MAX_SIZE]; - public: - explicit TagBuffer(int32_t tag); - - void startList(int8_t count); - void endList(); - - void writeInt32(int32_t); - void writeInt64(int64_t); - void writeString(const std::string_view&); - - void log(); - }; - - friend class Singleton<EventLog>; - EventLog(const EventLog&); - EventLog& operator =(const EventLog&); - - enum { LOGTAG_SF_FRAME_DUR = 60100 }; - void doLogFrameDurations(const std::string_view& name, const int32_t* durations, - size_t numDurations); -}; - -} // namespace android diff --git a/services/surfaceflinger/EventLog/EventLogTags.logtags b/services/surfaceflinger/EventLog/EventLogTags.logtags index 6c851dd09c..e68d9f5ec0 100644 --- a/services/surfaceflinger/EventLog/EventLogTags.logtags +++ b/services/surfaceflinger/EventLog/EventLogTags.logtags @@ -35,7 +35,6 @@ # 60100 - 60199 reserved for surfaceflinger -60100 sf_frame_dur (window|3),(dur0|1),(dur1|1),(dur2|1),(dur3|1),(dur4|1),(dur5|1),(dur6|1) 60110 sf_stop_bootanim (time|2|3) # NOTE - the range 1000000-2000000 is reserved for partners and others who diff --git a/services/surfaceflinger/FrameTracker.cpp b/services/surfaceflinger/FrameTracker.cpp index ca8cdc3c0a..93d0313e53 100644 --- a/services/surfaceflinger/FrameTracker.cpp +++ b/services/surfaceflinger/FrameTracker.cpp @@ -26,16 +26,10 @@ #include <ui/FrameStats.h> #include "FrameTracker.h" -#include "EventLog/EventLog.h" namespace android { -FrameTracker::FrameTracker() : - mOffset(0), - mNumFences(0), - mDisplayPeriod(0) { - resetFrameCountersLocked(); -} +FrameTracker::FrameTracker() : mOffset(0), mNumFences(0), mDisplayPeriod(0) {} void FrameTracker::setDesiredPresentTime(nsecs_t presentTime) { Mutex::Autolock lock(mMutex); @@ -73,9 +67,6 @@ void FrameTracker::setDisplayRefreshPeriod(nsecs_t displayPeriod) { void FrameTracker::advanceFrame() { Mutex::Autolock lock(mMutex); - // Update the statistic to include the frame we just finished. - updateStatsLocked(mOffset); - // Advance to the next frame. mOffset = (mOffset+1) % NUM_FRAME_RECORDS; mFrameRecords[mOffset].desiredPresentTime = INT64_MAX; @@ -138,19 +129,12 @@ void FrameTracker::getStats(FrameStats* outStats) const { } } -void FrameTracker::logAndResetStats(const std::string_view& name) { - Mutex::Autolock lock(mMutex); - logStatsLocked(name); - resetFrameCountersLocked(); -} - void FrameTracker::processFencesLocked() const { FrameRecord* records = const_cast<FrameRecord*>(mFrameRecords); int& numFences = const_cast<int&>(mNumFences); for (int i = 1; i < NUM_FRAME_RECORDS && numFences > 0; i++) { - size_t idx = (mOffset+NUM_FRAME_RECORDS-i) % NUM_FRAME_RECORDS; - bool updated = false; + size_t idx = (mOffset + NUM_FRAME_RECORDS - i) % NUM_FRAME_RECORDS; const std::shared_ptr<FenceTime>& rfence = records[idx].frameReadyFence; if (rfence != nullptr) { @@ -158,7 +142,6 @@ void FrameTracker::processFencesLocked() const { if (records[idx].frameReadyTime < INT64_MAX) { records[idx].frameReadyFence = nullptr; numFences--; - updated = true; } } @@ -169,58 +152,7 @@ void FrameTracker::processFencesLocked() const { if (records[idx].actualPresentTime < INT64_MAX) { records[idx].actualPresentFence = nullptr; numFences--; - updated = true; - } - } - - if (updated) { - updateStatsLocked(idx); - } - } -} - -void FrameTracker::updateStatsLocked(size_t newFrameIdx) const { - int* numFrames = const_cast<int*>(mNumFrames); - - if (mDisplayPeriod > 0 && isFrameValidLocked(newFrameIdx)) { - size_t prevFrameIdx = (newFrameIdx+NUM_FRAME_RECORDS-1) % - NUM_FRAME_RECORDS; - - if (isFrameValidLocked(prevFrameIdx)) { - nsecs_t newPresentTime = - mFrameRecords[newFrameIdx].actualPresentTime; - nsecs_t prevPresentTime = - mFrameRecords[prevFrameIdx].actualPresentTime; - - nsecs_t duration = newPresentTime - prevPresentTime; - int numPeriods = int((duration + mDisplayPeriod/2) / - mDisplayPeriod); - - for (int i = 0; i < NUM_FRAME_BUCKETS-1; i++) { - int nextBucket = 1 << (i+1); - if (numPeriods < nextBucket) { - numFrames[i]++; - return; - } } - - // The last duration bucket is a catch-all. - numFrames[NUM_FRAME_BUCKETS-1]++; - } - } -} - -void FrameTracker::resetFrameCountersLocked() { - for (int i = 0; i < NUM_FRAME_BUCKETS; i++) { - mNumFrames[i] = 0; - } -} - -void FrameTracker::logStatsLocked(const std::string_view& name) const { - for (int i = 0; i < NUM_FRAME_BUCKETS; i++) { - if (mNumFrames[i] > 0) { - EventLog::logFrameDurations(name, mNumFrames, NUM_FRAME_BUCKETS); - return; } } } diff --git a/services/surfaceflinger/FrameTracker.h b/services/surfaceflinger/FrameTracker.h index bc412aee2f..fd6fadc6dd 100644 --- a/services/surfaceflinger/FrameTracker.h +++ b/services/surfaceflinger/FrameTracker.h @@ -41,8 +41,6 @@ public: // frame time history. enum { NUM_FRAME_RECORDS = 128 }; - enum { NUM_FRAME_BUCKETS = 7 }; - FrameTracker(); // setDesiredPresentTime sets the time at which the current frame @@ -142,13 +140,6 @@ private: // doesn't grow with NUM_FRAME_RECORDS. int mNumFences; - // mNumFrames keeps a count of the number of frames with a duration in a - // particular range of vsync periods. Element n of the array stores the - // number of frames with duration in the half-inclusive range - // [2^n, 2^(n+1)). The last element of the array contains the count for - // all frames with duration greater than 2^(NUM_FRAME_BUCKETS-1). - int32_t mNumFrames[NUM_FRAME_BUCKETS]; - // mDisplayPeriod is the display refresh period of the display for which // this FrameTracker is gathering information. nsecs_t mDisplayPeriod; diff --git a/services/surfaceflinger/FrontEnd/LayerHandle.cpp b/services/surfaceflinger/FrontEnd/LayerHandle.cpp index 75e4e3ae11..ffd51a40bb 100644 --- a/services/surfaceflinger/FrontEnd/LayerHandle.cpp +++ b/services/surfaceflinger/FrontEnd/LayerHandle.cpp @@ -28,7 +28,7 @@ LayerHandle::LayerHandle(const sp<android::SurfaceFlinger>& flinger, LayerHandle::~LayerHandle() { if (mFlinger) { - mFlinger->onHandleDestroyed(this, mLayer, mLayerId); + mFlinger->onHandleDestroyed(mLayer, mLayerId); } } diff --git a/services/surfaceflinger/FrontEnd/LayerSnapshot.h b/services/surfaceflinger/FrontEnd/LayerSnapshot.h index 398e64a4f1..b7d4cc5d06 100644 --- a/services/surfaceflinger/FrontEnd/LayerSnapshot.h +++ b/services/surfaceflinger/FrontEnd/LayerSnapshot.h @@ -72,7 +72,7 @@ struct LayerSnapshot : public compositionengine::LayerFECompositionState { bool premultipliedAlpha; ui::Transform parentTransform; Rect bufferSize; - Rect croppedBufferSize; + FloatRect croppedBufferSize; std::shared_ptr<renderengine::ExternalTexture> externalTexture; gui::LayerMetadata layerMetadata; gui::LayerMetadata relativeLayerMetadata; diff --git a/services/surfaceflinger/FrontEnd/LayerSnapshotBuilder.cpp b/services/surfaceflinger/FrontEnd/LayerSnapshotBuilder.cpp index ee605b7a3e..10e212e7b5 100644 --- a/services/surfaceflinger/FrontEnd/LayerSnapshotBuilder.cpp +++ b/services/surfaceflinger/FrontEnd/LayerSnapshotBuilder.cpp @@ -116,7 +116,7 @@ ui::Transform getInputTransform(const LayerSnapshot& snapshot) { * that's already included. */ std::pair<FloatRect, bool> getInputBounds(const LayerSnapshot& snapshot, bool fillParentBounds) { - FloatRect inputBounds = snapshot.croppedBufferSize.toFloatRect(); + FloatRect inputBounds = snapshot.croppedBufferSize; if (snapshot.hasBufferOrSidebandStream() && snapshot.croppedBufferSize.isValid() && snapshot.localTransform.getType() != ui::Transform::IDENTITY) { inputBounds = snapshot.localTransform.transform(inputBounds); @@ -220,7 +220,7 @@ void handleDropInputMode(LayerSnapshot& snapshot, const LayerSnapshot& parentSna } // Check if the parent has cropped the buffer - Rect bufferSize = snapshot.croppedBufferSize; + FloatRect bufferSize = snapshot.croppedBufferSize; if (!bufferSize.isValid()) { snapshot.inputInfo.inputConfig |= gui::WindowInfo::InputConfig::DROP_INPUT_IF_OBSCURED; return; @@ -970,7 +970,7 @@ void LayerSnapshotBuilder::updateRoundedCorner(LayerSnapshot& snapshot, parentRoundedCorner.radius.y *= t.getScaleY(); } - FloatRect layerCropRect = snapshot.croppedBufferSize.toFloatRect(); + FloatRect layerCropRect = snapshot.croppedBufferSize; const vec2 radius(requested.cornerRadius, requested.cornerRadius); RoundedCornerState layerSettings(layerCropRect, radius); const bool layerSettingsValid = layerSettings.hasRoundedCorners() && !layerCropRect.isEmpty(); @@ -1061,7 +1061,7 @@ void LayerSnapshotBuilder::updateLayerBounds(LayerSnapshot& snapshot, requested.externalTexture ? snapshot.bufferSize.toFloatRect() : parentBounds; snapshot.geomLayerCrop = parentBounds; if (!requested.crop.isEmpty()) { - snapshot.geomLayerCrop = snapshot.geomLayerCrop.intersect(requested.crop.toFloatRect()); + snapshot.geomLayerCrop = snapshot.geomLayerCrop.intersect(requested.crop); } snapshot.geomLayerBounds = snapshot.geomLayerBounds.intersect(snapshot.geomLayerCrop); snapshot.transformedBounds = snapshot.geomLayerTransform.transform(snapshot.geomLayerBounds); @@ -1072,10 +1072,10 @@ void LayerSnapshotBuilder::updateLayerBounds(LayerSnapshot& snapshot, snapshot.geomLayerTransform.transform(geomLayerBoundsWithoutTransparentRegion); snapshot.parentTransform = parentSnapshot.geomLayerTransform; - // Subtract the transparent region and snap to the bounds - const Rect bounds = - RequestedLayerState::reduce(snapshot.croppedBufferSize, requested.transparentRegion); if (requested.potentialCursor) { + // Subtract the transparent region and snap to the bounds + const Rect bounds = RequestedLayerState::reduce(Rect(snapshot.croppedBufferSize), + requested.transparentRegion); snapshot.cursorFrame = snapshot.geomLayerTransform.transform(bounds); } } @@ -1192,7 +1192,8 @@ void LayerSnapshotBuilder::updateInput(LayerSnapshot& snapshot, snapshot.inputInfo.inputConfig |= InputConfig::TRUSTED_OVERLAY; } - snapshot.inputInfo.contentSize = snapshot.croppedBufferSize.getSize(); + snapshot.inputInfo.contentSize = {snapshot.croppedBufferSize.getHeight(), + snapshot.croppedBufferSize.getWidth()}; // If the layer is a clone, we need to crop the input region to cloned root to prevent // touches from going outside the cloned area. diff --git a/services/surfaceflinger/FrontEnd/RequestedLayerState.cpp b/services/surfaceflinger/FrontEnd/RequestedLayerState.cpp index 5734ccf38f..1eff9d699c 100644 --- a/services/surfaceflinger/FrontEnd/RequestedLayerState.cpp +++ b/services/surfaceflinger/FrontEnd/RequestedLayerState.cpp @@ -96,7 +96,7 @@ RequestedLayerState::RequestedLayerState(const LayerCreationArgs& args) LLOGV(layerId, "Created %s flags=%d", getDebugString().c_str(), flags); color.a = 1.0f; - crop.makeInvalid(); + crop = {0, 0, -1, -1}; z = 0; layerStack = ui::DEFAULT_LAYER_STACK; transformToDisplayInverse = false; @@ -473,10 +473,10 @@ Rect RequestedLayerState::getBufferSize(uint32_t displayRotationFlags) const { return Rect(0, 0, static_cast<int32_t>(bufWidth), static_cast<int32_t>(bufHeight)); } -Rect RequestedLayerState::getCroppedBufferSize(const Rect& bufferSize) const { - Rect size = bufferSize; +FloatRect RequestedLayerState::getCroppedBufferSize(const Rect& bufferSize) const { + FloatRect size = bufferSize.toFloatRect(); if (!crop.isEmpty() && size.isValid()) { - size.intersect(crop, &size); + size = size.intersect(crop); } else if (!crop.isEmpty()) { size = crop; } diff --git a/services/surfaceflinger/FrontEnd/RequestedLayerState.h b/services/surfaceflinger/FrontEnd/RequestedLayerState.h index 1d96dff336..3220e86ce4 100644 --- a/services/surfaceflinger/FrontEnd/RequestedLayerState.h +++ b/services/surfaceflinger/FrontEnd/RequestedLayerState.h @@ -79,7 +79,7 @@ struct RequestedLayerState : layer_state_t { bool isHiddenByPolicy() const; half4 getColor() const; Rect getBufferSize(uint32_t displayRotationFlags) const; - Rect getCroppedBufferSize(const Rect& bufferSize) const; + FloatRect getCroppedBufferSize(const Rect& bufferSize) const; Rect getBufferCrop() const; std::string getDebugString() const; std::string getDebugStringShort() const; diff --git a/services/surfaceflinger/Layer.cpp b/services/surfaceflinger/Layer.cpp index dcb0812b67..92894847fa 100644 --- a/services/surfaceflinger/Layer.cpp +++ b/services/surfaceflinger/Layer.cpp @@ -138,7 +138,7 @@ Layer::Layer(const surfaceflinger::LayerCreationArgs& args) args.metadata.getInt32(gui::METADATA_WINDOW_TYPE, 0))) { ALOGV("Creating Layer %s", getDebugName()); - mDrawingState.crop.makeInvalid(); + mDrawingState.crop = {0, 0, -1, -1}; mDrawingState.sequence = 0; mDrawingState.transform.set(0, 0); mDrawingState.frameNumber = 0; @@ -183,7 +183,6 @@ Layer::~Layer() { mFlinger->mTimeStats->onDestroy(layerId); mFlinger->mFrameTracer->onDestroy(layerId); - mFrameTracker.logAndResetStats(mName); mFlinger->onLayerDestroyed(this); if (mDrawingState.sidebandStream != nullptr) { @@ -316,7 +315,7 @@ bool Layer::computeTrustedPresentationState(const FloatRect& bounds, const Float Rect Layer::getCroppedBufferSize(const State& s) const { Rect size = getBufferSize(s); - Rect crop = getCrop(s); + Rect crop = Rect(getCrop(s)); if (!crop.isEmpty() && size.isValid()) { size.intersect(crop, &size); } else if (!crop.isEmpty()) { @@ -373,7 +372,7 @@ void Layer::setTransactionFlags(uint32_t mask) { mTransactionFlags |= mask; } -bool Layer::setCrop(const Rect& crop) { +bool Layer::setCrop(const FloatRect& crop) { if (mDrawingState.crop == crop) return false; mDrawingState.sequence++; mDrawingState.crop = crop; @@ -605,10 +604,6 @@ void Layer::clearFrameStats() { mFrameTracker.clearStats(); } -void Layer::logFrameStats() { - mFrameTracker.logAndResetStats(mName); -} - void Layer::getFrameStats(FrameStats* outStats) const { mFrameTracker.getStats(outStats); } diff --git a/services/surfaceflinger/Layer.h b/services/surfaceflinger/Layer.h index 9bc557e917..57093ae602 100644 --- a/services/surfaceflinger/Layer.h +++ b/services/surfaceflinger/Layer.h @@ -95,7 +95,7 @@ public: struct State { int32_t sequence; // changes when visible regions can change // Crop is expressed in layer space coordinate. - Rect crop; + FloatRect crop; LayerMetadata metadata; ui::Dataspace dataspace; @@ -172,7 +172,7 @@ public: // be delayed until the resize completes. // Buffer space - bool setCrop(const Rect& crop); + bool setCrop(const FloatRect& crop); bool setTransform(uint32_t /*transform*/); bool setTransformToDisplayInverse(bool /*transformToDisplayInverse*/); @@ -198,7 +198,7 @@ public: Region getVisibleRegion(const DisplayDevice*) const; void updateLastLatchTime(nsecs_t latchtime); - Rect getCrop(const Layer::State& s) const { return s.crop; } + Rect getCrop(const Layer::State& s) const { return Rect(s.crop); } // from graphics API static ui::Dataspace translateDataspace(ui::Dataspace dataspace); diff --git a/services/surfaceflinger/LayerProtoHelper.cpp b/services/surfaceflinger/LayerProtoHelper.cpp index 5eea45b436..a0fbab0728 100644 --- a/services/surfaceflinger/LayerProtoHelper.cpp +++ b/services/surfaceflinger/LayerProtoHelper.cpp @@ -106,6 +106,13 @@ void LayerProtoHelper::readFromProto(const perfetto::protos::RectProto& proto, R outRect.right = proto.right(); } +void LayerProtoHelper::readFromProto(const perfetto::protos::RectProto& proto, FloatRect& outRect) { + outRect.left = proto.left(); + outRect.top = proto.top(); + outRect.bottom = proto.bottom(); + outRect.right = proto.right(); +} + void LayerProtoHelper::writeToProto( const FloatRect& rect, std::function<perfetto::protos::FloatRectProto*()> getFloatRectProto) { @@ -427,7 +434,7 @@ void LayerProtoHelper::writeSnapshotToProto(perfetto::protos::LayerProto* layerI layerInfo->mutable_color_transform()); } - LayerProtoHelper::writeToProto(snapshot.croppedBufferSize.toFloatRect(), + LayerProtoHelper::writeToProto(snapshot.croppedBufferSize, [&]() { return layerInfo->mutable_source_bounds(); }); LayerProtoHelper::writeToProto(snapshot.transformedBounds, [&]() { return layerInfo->mutable_screen_bounds(); }); @@ -455,7 +462,7 @@ void LayerProtoHelper::writeSnapshotToProto(perfetto::protos::LayerProto* layerI return layerInfo->mutable_requested_position(); }); - LayerProtoHelper::writeToProto(requestedState.crop, + LayerProtoHelper::writeToProto(Rect(requestedState.crop), [&]() { return layerInfo->mutable_crop(); }); layerInfo->set_is_opaque(snapshot.contentOpaque); diff --git a/services/surfaceflinger/LayerProtoHelper.h b/services/surfaceflinger/LayerProtoHelper.h index 41ea68420f..3ca553a903 100644 --- a/services/surfaceflinger/LayerProtoHelper.h +++ b/services/surfaceflinger/LayerProtoHelper.h @@ -44,6 +44,7 @@ public: std::function<perfetto::protos::RectProto*()> getRectProto); static void writeToProto(const Rect& rect, perfetto::protos::RectProto* rectProto); static void readFromProto(const perfetto::protos::RectProto& proto, Rect& outRect); + static void readFromProto(const perfetto::protos::RectProto& proto, FloatRect& outRect); static void writeToProto(const FloatRect& rect, std::function<perfetto::protos::FloatRectProto*()> getFloatRectProto); static void writeToProto(const Region& region, diff --git a/services/surfaceflinger/OWNERS b/services/surfaceflinger/OWNERS index ffc1dd7979..fa0ecee6b5 100644 --- a/services/surfaceflinger/OWNERS +++ b/services/surfaceflinger/OWNERS @@ -5,6 +5,8 @@ alecmouri@google.com domlaskowski@google.com jreck@google.com lpy@google.com +mattbuckley@google.com +melodymhsu@google.com pdwilliams@google.com racarr@google.com ramindani@google.com @@ -12,3 +14,4 @@ rnlee@google.com sallyqi@google.com scroggo@google.com vishnun@google.com +xwxw@google.com diff --git a/services/surfaceflinger/Scheduler/EventThread.cpp b/services/surfaceflinger/Scheduler/EventThread.cpp index 218c56ef3d..e385f18243 100644 --- a/services/surfaceflinger/Scheduler/EventThread.cpp +++ b/services/surfaceflinger/Scheduler/EventThread.cpp @@ -426,8 +426,8 @@ void EventThread::onVsync(nsecs_t vsyncTime, nsecs_t wakeupTime, nsecs_t readyTi LOG_FATAL_IF(!mVSyncState); mVsyncTracer = (mVsyncTracer + 1) % 2; - mPendingEvents.push_back(makeVSync(mVSyncState->displayId, wakeupTime, ++mVSyncState->count, - vsyncTime, readyTime)); + mPendingEvents.push_back(makeVSync(mVsyncSchedule->getPhysicalDisplayId(), wakeupTime, + ++mVSyncState->count, vsyncTime, readyTime)); mCondition.notify_all(); } @@ -486,9 +486,9 @@ void EventThread::threadMain(std::unique_lock<std::mutex>& lock) { if (event->header.type == DisplayEventReceiver::DISPLAY_EVENT_HOTPLUG) { if (event->hotplug.connectionError == 0) { if (event->hotplug.connected && !mVSyncState) { - mVSyncState.emplace(event->header.displayId); - } else if (!event->hotplug.connected && mVSyncState && - mVSyncState->displayId == event->header.displayId) { + mVSyncState.emplace(); + } else if (!event->hotplug.connected && + mVsyncSchedule->getPhysicalDisplayId() == event->header.displayId) { mVSyncState.reset(); } } else { @@ -559,7 +559,7 @@ void EventThread::threadMain(std::unique_lock<std::mutex>& lock) { const auto now = systemTime(SYSTEM_TIME_MONOTONIC); const auto deadlineTimestamp = now + timeout.count(); const auto expectedVSyncTime = deadlineTimestamp + timeout.count(); - mPendingEvents.push_back(makeVSync(mVSyncState->displayId, now, + mPendingEvents.push_back(makeVSync(mVsyncSchedule->getPhysicalDisplayId(), now, ++mVSyncState->count, expectedVSyncTime, deadlineTimestamp)); } @@ -739,7 +739,7 @@ void EventThread::dump(std::string& result) const { StringAppendF(&result, "%s: state=%s VSyncState=", mThreadName, toCString(mState)); if (mVSyncState) { StringAppendF(&result, "{displayId=%s, count=%u%s}\n", - to_string(mVSyncState->displayId).c_str(), mVSyncState->count, + to_string(mVsyncSchedule->getPhysicalDisplayId()).c_str(), mVSyncState->count, mVSyncState->synthetic ? ", synthetic" : ""); } else { StringAppendF(&result, "none\n"); diff --git a/services/surfaceflinger/Scheduler/EventThread.h b/services/surfaceflinger/Scheduler/EventThread.h index bbe4f9d899..c3c7eb0ee1 100644 --- a/services/surfaceflinger/Scheduler/EventThread.h +++ b/services/surfaceflinger/Scheduler/EventThread.h @@ -235,10 +235,6 @@ private: // VSYNC state of connected display. struct VSyncState { - explicit VSyncState(PhysicalDisplayId displayId) : displayId(displayId) {} - - const PhysicalDisplayId displayId; - // Number of VSYNC events since display was connected. uint32_t count = 0; diff --git a/services/surfaceflinger/Scheduler/VsyncSchedule.h b/services/surfaceflinger/Scheduler/VsyncSchedule.h index 881d6789b2..e63cbb2c82 100644 --- a/services/surfaceflinger/Scheduler/VsyncSchedule.h +++ b/services/surfaceflinger/Scheduler/VsyncSchedule.h @@ -112,6 +112,8 @@ public: bool getPendingHardwareVsyncState() const REQUIRES(kMainThreadContext); + PhysicalDisplayId getPhysicalDisplayId() const { return mId; } + protected: using ControllerPtr = std::unique_ptr<VsyncController>; diff --git a/services/surfaceflinger/SurfaceFlinger.cpp b/services/surfaceflinger/SurfaceFlinger.cpp index 06560d06ba..5010ba15bf 100644 --- a/services/surfaceflinger/SurfaceFlinger.cpp +++ b/services/surfaceflinger/SurfaceFlinger.cpp @@ -952,7 +952,7 @@ void SurfaceFlinger::init() FTL_FAKE_GUARD(kMainThreadContext) { // Do not wait the future to avoid deadlocks // between main and Perfetto threads (b/313130597) static_cast<void>(mScheduler->schedule( - [&, onLayersSnapshot]() FTL_FAKE_GUARD(mStateLock) + [&, traceFlags, onLayersSnapshot]() FTL_FAKE_GUARD(mStateLock) FTL_FAKE_GUARD(kMainThreadContext) { auto snapshot = takeLayersSnapshotProto(traceFlags, TimePoint::now(), @@ -1654,6 +1654,22 @@ status_t SurfaceFlinger::getOverlaySupport(gui::OverlayProperties* outProperties outProperties->combinations.emplace_back(outCombination); } outProperties->supportMixedColorSpaces = aidlProperties.supportMixedColorSpaces; + if (aidlProperties.lutProperties.has_value()) { + std::vector<gui::LutProperties> outLutProperties; + for (const auto& properties : aidlProperties.lutProperties.value()) { + gui::LutProperties currentProperties; + currentProperties.dimension = + static_cast<gui::LutProperties::Dimension>(properties->dimension); + currentProperties.size = properties->size; + currentProperties.samplingKeys.reserve(properties->samplingKeys.size()); + std::transform(properties->samplingKeys.cbegin(), properties->samplingKeys.cend(), + std::back_inserter(currentProperties.samplingKeys), [](const auto& val) { + return static_cast<gui::LutProperties::SamplingKey>(val); + }); + outLutProperties.push_back(std::move(currentProperties)); + } + outProperties->lutProperties.emplace(outLutProperties.begin(), outLutProperties.end()); + } return NO_ERROR; } @@ -3268,8 +3284,6 @@ void SurfaceFlinger::onCompositionPresented(PhysicalDisplayId pacesetterId, // getTotalSize returns the total number of buffers that were allocated by SurfaceFlinger SFTRACE_INT64("Total Buffer Size", GraphicBufferAllocator::get().getTotalSize()); } - - logFrameStats(presentTime); } void SurfaceFlinger::commitTransactions() { @@ -4723,6 +4737,7 @@ status_t SurfaceFlinger::setTransactionState( for (auto& state : states) { resolvedStates.emplace_back(std::move(state)); auto& resolvedState = resolvedStates.back(); + resolvedState.layerId = LayerHandle::getLayerId(resolvedState.state.surface); if (resolvedState.state.hasBufferChanges() && resolvedState.state.hasValidBuffer() && resolvedState.state.surface) { sp<Layer> layer = LayerHandle::getLayer(resolvedState.state.surface); @@ -4734,9 +4749,8 @@ status_t SurfaceFlinger::setTransactionState( if (resolvedState.externalTexture) { resolvedState.state.bufferData->buffer = resolvedState.externalTexture->getBuffer(); } - mBufferCountTracker.increment(resolvedState.state.surface->localBinder()); + mBufferCountTracker.increment(resolvedState.layerId); } - resolvedState.layerId = LayerHandle::getLayerId(resolvedState.state.surface); if (resolvedState.state.what & layer_state_t::eReparent) { resolvedState.parentId = getLayerIdFromSurfaceControl(resolvedState.state.parentSurfaceControlForChild); @@ -5179,7 +5193,7 @@ status_t SurfaceFlinger::createLayer(LayerCreationArgs& args, gui::CreateSurface std::atomic<int32_t>* pendingBufferCounter = layer->getPendingBufferCounter(); if (pendingBufferCounter) { std::string counterName = layer->getPendingBufferCounterName(); - mBufferCountTracker.add(outResult.handle->localBinder(), counterName, + mBufferCountTracker.add(LayerHandle::getLayerId(outResult.handle), counterName, pendingBufferCounter); } } break; @@ -5227,7 +5241,7 @@ status_t SurfaceFlinger::createEffectLayer(const LayerCreationArgs& args, sp<IBi return NO_ERROR; } -void SurfaceFlinger::onHandleDestroyed(BBinder* handle, sp<Layer>& layer, uint32_t layerId) { +void SurfaceFlinger::onHandleDestroyed(sp<Layer>& layer, uint32_t layerId) { { // Used to remove stalled transactions which uses an internal lock. ftl::FakeGuard guard(kMainThreadContext); @@ -5240,7 +5254,7 @@ void SurfaceFlinger::onHandleDestroyed(BBinder* handle, sp<Layer>& layer, uint32 Mutex::Autolock stateLock(mStateLock); layer->onHandleDestroyed(); - mBufferCountTracker.remove(handle); + mBufferCountTracker.remove(layerId); layer.clear(); setTransactionFlags(eTransactionFlushNeeded | eTransactionNeeded); } @@ -5741,7 +5755,7 @@ void SurfaceFlinger::dumpHdrInfo(std::string& result) const { void SurfaceFlinger::dumpFrontEnd(std::string& result) { std::ostringstream out; - out << "\nComposition list\n"; + out << "\nComposition list (bottom to top)\n"; ui::LayerStack lastPrintedLayerStackHeader = ui::INVALID_LAYER_STACK; for (const auto& snapshot : mLayerSnapshotBuilder.getSnapshots()) { if (lastPrintedLayerStackHeader != snapshot->outputFilter.layerStack) { @@ -5769,7 +5783,7 @@ void SurfaceFlinger::dumpFrontEnd(std::string& result) { void SurfaceFlinger::dumpVisibleFrontEnd(std::string& result) { std::ostringstream out; - out << "\nComposition list\n"; + out << "\nComposition list (bottom to top)\n"; ui::LayerStack lastPrintedLayerStackHeader = ui::INVALID_LAYER_STACK; mLayerSnapshotBuilder.forEachVisibleSnapshot( [&](std::unique_ptr<frontend::LayerSnapshot>& snapshot) { diff --git a/services/surfaceflinger/SurfaceFlinger.h b/services/surfaceflinger/SurfaceFlinger.h index 3eb72cc4c0..db0e15e6c2 100644 --- a/services/surfaceflinger/SurfaceFlinger.h +++ b/services/surfaceflinger/SurfaceFlinger.h @@ -296,7 +296,7 @@ public: // Called when all clients have released all their references to // this layer. The layer may still be kept alive by its parents but // the client can no longer modify this layer directly. - void onHandleDestroyed(BBinder* handle, sp<Layer>& layer, uint32_t layerId); + void onHandleDestroyed(sp<Layer>& layer, uint32_t layerId); TransactionCallbackInvoker& getTransactionCallbackInvoker() { return mTransactionCallbackInvoker; @@ -433,32 +433,32 @@ private: // This is done to avoid lock contention with the main thread. class BufferCountTracker { public: - void increment(BBinder* layerHandle) { + void increment(uint32_t layerId) { std::lock_guard<std::mutex> lock(mLock); - auto it = mCounterByLayerHandle.find(layerHandle); - if (it != mCounterByLayerHandle.end()) { + auto it = mCounterByLayerId.find(layerId); + if (it != mCounterByLayerId.end()) { auto [name, pendingBuffers] = it->second; int32_t count = ++(*pendingBuffers); SFTRACE_INT(name.c_str(), count); } else { - ALOGW("Handle not found! %p", layerHandle); + ALOGW("Layer ID not found! %d", layerId); } } - void add(BBinder* layerHandle, const std::string& name, std::atomic<int32_t>* counter) { + void add(uint32_t layerId, const std::string& name, std::atomic<int32_t>* counter) { std::lock_guard<std::mutex> lock(mLock); - mCounterByLayerHandle[layerHandle] = std::make_pair(name, counter); + mCounterByLayerId[layerId] = std::make_pair(name, counter); } - void remove(BBinder* layerHandle) { + void remove(uint32_t layerId) { std::lock_guard<std::mutex> lock(mLock); - mCounterByLayerHandle.erase(layerHandle); + mCounterByLayerId.erase(layerId); } private: std::mutex mLock; - std::unordered_map<BBinder*, std::pair<std::string, std::atomic<int32_t>*>> - mCounterByLayerHandle GUARDED_BY(mLock); + std::unordered_map<uint32_t, std::pair<std::string, std::atomic<int32_t>*>> + mCounterByLayerId GUARDED_BY(mLock); }; enum class BootStage { diff --git a/services/surfaceflinger/Tracing/TransactionProtoParser.cpp b/services/surfaceflinger/Tracing/TransactionProtoParser.cpp index b1895989ec..f39a4d2af4 100644 --- a/services/surfaceflinger/Tracing/TransactionProtoParser.cpp +++ b/services/surfaceflinger/Tracing/TransactionProtoParser.cpp @@ -147,7 +147,7 @@ perfetto::protos::LayerState TransactionProtoParser::toProto( proto.set_transform_to_display_inverse(layer.transformToDisplayInverse); } if (layer.what & layer_state_t::eCropChanged) { - LayerProtoHelper::writeToProto(layer.crop, proto.mutable_crop()); + LayerProtoHelper::writeToProto(Rect(layer.crop), proto.mutable_crop()); } if (layer.what & layer_state_t::eBufferChanged) { perfetto::protos::LayerState_BufferData* bufferProto = proto.mutable_buffer_data(); diff --git a/services/surfaceflinger/tests/benchmarks/Android.bp b/services/surfaceflinger/tests/benchmarks/Android.bp index 1c47be343e..22fca08ccf 100644 --- a/services/surfaceflinger/tests/benchmarks/Android.bp +++ b/services/surfaceflinger/tests/benchmarks/Android.bp @@ -22,7 +22,6 @@ cc_benchmark { static_libs: [ "libgmock", "libgtest", - "libc++fs", ], header_libs: [ "libsurfaceflinger_mocks_headers", diff --git a/services/surfaceflinger/tests/common/LayerLifecycleManagerHelper.h b/services/surfaceflinger/tests/common/LayerLifecycleManagerHelper.h index ae380ad459..b472047bd6 100644 --- a/services/surfaceflinger/tests/common/LayerLifecycleManagerHelper.h +++ b/services/surfaceflinger/tests/common/LayerLifecycleManagerHelper.h @@ -182,7 +182,7 @@ public: mLifecycleManager.applyTransactions(setZTransaction(id, z)); } - void setCrop(uint32_t id, const Rect& crop) { + void setCrop(uint32_t id, const FloatRect& crop) { std::vector<TransactionState> transactions; transactions.emplace_back(); transactions.back().states.push_back({}); @@ -193,6 +193,8 @@ public: mLifecycleManager.applyTransactions(transactions); } + void setCrop(uint32_t id, const Rect& crop) { setCrop(id, crop.toFloatRect()); } + void setFlags(uint32_t id, uint32_t mask, uint32_t flags) { std::vector<TransactionState> transactions; transactions.emplace_back(); diff --git a/services/surfaceflinger/tests/unittests/CompositionTest.cpp b/services/surfaceflinger/tests/unittests/CompositionTest.cpp index 23d3c168bd..4f72424bd7 100644 --- a/services/surfaceflinger/tests/unittests/CompositionTest.cpp +++ b/services/surfaceflinger/tests/unittests/CompositionTest.cpp @@ -467,7 +467,7 @@ struct BaseLayerProperties { LayerProperties::FORMAT, LayerProperties::USAGE | GraphicBuffer::USAGE_HW_TEXTURE); - layer.crop = Rect(0, 0, LayerProperties::HEIGHT, LayerProperties::WIDTH); + layer.crop = FloatRect(0, 0, LayerProperties::HEIGHT, LayerProperties::WIDTH); layer.externalTexture = buffer; layer.bufferData->acquireFence = Fence::NO_FENCE; layer.dataspace = ui::Dataspace::UNKNOWN; @@ -664,7 +664,8 @@ struct SidebandLayerProperties : public BaseLayerProperties<SidebandLayerPropert NativeHandle::create(reinterpret_cast<native_handle_t*>(DEFAULT_SIDEBAND_STREAM), false); layer.sidebandStream = stream; - layer.crop = Rect(0, 0, SidebandLayerProperties::HEIGHT, SidebandLayerProperties::WIDTH); + layer.crop = + FloatRect(0, 0, SidebandLayerProperties::HEIGHT, SidebandLayerProperties::WIDTH); } static void setupHwcSetSourceCropBufferCallExpectations(CompositionTest* test) { @@ -828,7 +829,7 @@ struct EffectLayerVariant : public BaseLayerVariant<LayerProperties> { return frontend::RequestedLayerState(args); }); - layer.crop = Rect(0, 0, LayerProperties::HEIGHT, LayerProperties::WIDTH); + layer.crop = FloatRect(0, 0, LayerProperties::HEIGHT, LayerProperties::WIDTH); return layer; } diff --git a/services/surfaceflinger/tests/unittests/LayerSnapshotTest.cpp b/services/surfaceflinger/tests/unittests/LayerSnapshotTest.cpp index 75d2fa3c7f..a35ae15c03 100644 --- a/services/surfaceflinger/tests/unittests/LayerSnapshotTest.cpp +++ b/services/surfaceflinger/tests/unittests/LayerSnapshotTest.cpp @@ -162,12 +162,12 @@ TEST_F(LayerSnapshotTest, croppedByParent) { info.info.logicalHeight = 100; info.info.logicalWidth = 200; mFrontEndDisplayInfos.emplace_or_replace(ui::LayerStack::fromValue(1), info); - Rect layerCrop(0, 0, 10, 20); + FloatRect layerCrop(0, 0, 10, 20); setCrop(11, layerCrop); EXPECT_TRUE(mLifecycleManager.getGlobalChanges().test(RequestedLayerState::Changes::Geometry)); UPDATE_AND_VERIFY_WITH_DISPLAY_CHANGES(mSnapshotBuilder, STARTING_ZORDER); EXPECT_EQ(getSnapshot(11)->geomCrop, layerCrop); - EXPECT_EQ(getSnapshot(111)->geomLayerBounds, layerCrop.toFloatRect()); + EXPECT_EQ(getSnapshot(111)->geomLayerBounds, layerCrop); float maxHeight = static_cast<float>(info.info.logicalHeight * 10); float maxWidth = static_cast<float>(info.info.logicalWidth * 10); diff --git a/services/surfaceflinger/tests/unittests/RefreshRateSelectorTest.cpp b/services/surfaceflinger/tests/unittests/RefreshRateSelectorTest.cpp index 9efe73d450..adbd868a28 100644 --- a/services/surfaceflinger/tests/unittests/RefreshRateSelectorTest.cpp +++ b/services/surfaceflinger/tests/unittests/RefreshRateSelectorTest.cpp @@ -100,7 +100,9 @@ struct TestableRefreshRateSelector : RefreshRateSelector { const std::vector<Fps>& knownFrameRates() const { return mKnownFrameRates; } using RefreshRateSelector::GetRankedFrameRatesCache; - auto& mutableGetRankedRefreshRatesCache() { return mGetRankedFrameRatesCache; } + auto& mutableGetRankedRefreshRatesCache() NO_THREAD_SAFETY_ANALYSIS { + return mGetRankedFrameRatesCache; + } auto getRankedFrameRates(const std::vector<LayerRequirement>& layers, GlobalSignals signals = {}, Fps pacesetterFps = {}) const { @@ -138,7 +140,9 @@ struct TestableRefreshRateSelector : RefreshRateSelector { return setPolicy(policy); } - const auto& getPrimaryFrameRates() const { return mPrimaryFrameRates; } + const auto& getPrimaryFrameRates() const NO_THREAD_SAFETY_ANALYSIS { + return mPrimaryFrameRates; + } }; class RefreshRateSelectorTest : public testing::TestWithParam<Config::FrameRateOverride> { diff --git a/services/surfaceflinger/tests/unittests/TestableSurfaceFlinger.h b/services/surfaceflinger/tests/unittests/TestableSurfaceFlinger.h index c043b880ec..4dec5f6b6a 100644 --- a/services/surfaceflinger/tests/unittests/TestableSurfaceFlinger.h +++ b/services/surfaceflinger/tests/unittests/TestableSurfaceFlinger.h @@ -637,7 +637,7 @@ public: void destroyAllLayerHandles() { ftl::FakeGuard guard(kMainThreadContext); for (auto [layerId, legacyLayer] : mFlinger->mLegacyLayers) { - mFlinger->onHandleDestroyed(nullptr, legacyLayer, layerId); + mFlinger->onHandleDestroyed(legacyLayer, layerId); } } diff --git a/services/surfaceflinger/tests/unittests/TransactionApplicationTest.cpp b/services/surfaceflinger/tests/unittests/TransactionApplicationTest.cpp index fab1f6d451..1e8cd0a0ca 100644 --- a/services/surfaceflinger/tests/unittests/TransactionApplicationTest.cpp +++ b/services/surfaceflinger/tests/unittests/TransactionApplicationTest.cpp @@ -387,7 +387,7 @@ public: state.state.what = what; if (what & layer_state_t::eCropChanged) { - state.state.crop = Rect(1, 2, 3, 4); + state.state.crop = FloatRect(1, 2, 3, 4); } if (what & layer_state_t::eFlagsChanged) { state.state.flags = layer_state_t::eEnableBackpressure; diff --git a/services/surfaceflinger/tests/unittests/mock/DisplayHardware/MockComposer.h b/services/surfaceflinger/tests/unittests/mock/DisplayHardware/MockComposer.h index f472d8fefe..615cc948ed 100644 --- a/services/surfaceflinger/tests/unittests/mock/DisplayHardware/MockComposer.h +++ b/services/surfaceflinger/tests/unittests/mock/DisplayHardware/MockComposer.h @@ -182,7 +182,7 @@ public: MOCK_METHOD(Error, notifyExpectedPresent, (Display, nsecs_t, int32_t)); MOCK_METHOD( Error, getRequestedLuts, - (Display, + (Display, std::vector<Layer>*, std::vector<aidl::android::hardware::graphics::composer3::DisplayLuts::LayerLut>*)); MOCK_METHOD(Error, setLayerLuts, (Display, Layer, std::vector<aidl::android::hardware::graphics::composer3::Lut>&)); diff --git a/services/surfaceflinger/tests/unittests/mock/DisplayHardware/MockHWC2.h b/services/surfaceflinger/tests/unittests/mock/DisplayHardware/MockHWC2.h index 5edd2cd9e3..53ed2e1f20 100644 --- a/services/surfaceflinger/tests/unittests/mock/DisplayHardware/MockHWC2.h +++ b/services/surfaceflinger/tests/unittests/mock/DisplayHardware/MockHWC2.h @@ -111,7 +111,7 @@ public: (aidl::android::hardware::graphics::composer3::OverlayProperties *), (const override)); MOCK_METHOD(hal::Error, getRequestedLuts, - (std::vector<aidl::android::hardware::graphics::composer3::DisplayLuts::LayerLut>*), + ((HWC2::Display::LayerLuts*), (HWC2::Display::LutFileDescriptorMapper&)), (override)); }; diff --git a/services/vibratorservice/VibratorHalWrapper.cpp b/services/vibratorservice/VibratorHalWrapper.cpp index 4ac16188fa..b06ee3b2b5 100644 --- a/services/vibratorservice/VibratorHalWrapper.cpp +++ b/services/vibratorservice/VibratorHalWrapper.cpp @@ -107,6 +107,10 @@ Info HalWrapper::getInfo() { mInfoCache.mMaxEnvelopeEffectControlPointDuration = getMaxEnvelopeEffectControlPointDurationInternal(); } + if (mInfoCache.mFrequencyToOutputAccelerationMap.isFailed()) { + mInfoCache.mFrequencyToOutputAccelerationMap = + getFrequencyToOutputAccelerationMapInternal(); + } return mInfoCache.get(); } @@ -239,6 +243,13 @@ HalResult<milliseconds> HalWrapper::getMaxEnvelopeEffectControlPointDurationInte return HalResult<milliseconds>::unsupported(); } +HalResult<std::vector<PwleV2OutputMapEntry>> +HalWrapper::getFrequencyToOutputAccelerationMapInternal() { + ALOGV("Skipped getFrequencyToOutputAccelerationMapInternal because it's not " + "available in Vibrator HAL"); + return HalResult<std::vector<PwleV2OutputMapEntry>>::unsupported(); +} + // ------------------------------------------------------------------------------------------------- HalResult<void> AidlHalWrapper::ping() { @@ -487,6 +498,15 @@ HalResult<milliseconds> AidlHalWrapper::getMaxEnvelopeEffectControlPointDuration return HalResultFactory::fromStatus<milliseconds>(std::move(status), milliseconds(durationMs)); } +HalResult<std::vector<PwleV2OutputMapEntry>> +AidlHalWrapper::getFrequencyToOutputAccelerationMapInternal() { + std::vector<PwleV2OutputMapEntry> frequencyToOutputAccelerationMap; + auto status = + getHal()->getPwleV2FrequencyToOutputAccelerationMap(&frequencyToOutputAccelerationMap); + return HalResultFactory::fromStatus< + std::vector<PwleV2OutputMapEntry>>(std::move(status), frequencyToOutputAccelerationMap); +} + std::shared_ptr<Aidl::IVibrator> AidlHalWrapper::getHal() { std::lock_guard<std::mutex> lock(mHandleMutex); return mHandle; diff --git a/services/vibratorservice/include/vibratorservice/VibratorHalWrapper.h b/services/vibratorservice/include/vibratorservice/VibratorHalWrapper.h index 4938b156e7..b2bfffca1b 100644 --- a/services/vibratorservice/include/vibratorservice/VibratorHalWrapper.h +++ b/services/vibratorservice/include/vibratorservice/VibratorHalWrapper.h @@ -243,6 +243,7 @@ public: using EffectStrength = aidl::android::hardware::vibrator::EffectStrength; using CompositePrimitive = aidl::android::hardware::vibrator::CompositePrimitive; using Braking = aidl::android::hardware::vibrator::Braking; + using PwleV2OutputMapEntry = aidl::android::hardware::vibrator::PwleV2OutputMapEntry; const HalResult<Capabilities> capabilities; const HalResult<std::vector<Effect>> supportedEffects; @@ -261,6 +262,7 @@ public: const HalResult<int32_t> maxEnvelopeEffectSize; const HalResult<std::chrono::milliseconds> minEnvelopeEffectControlPointDuration; const HalResult<std::chrono::milliseconds> maxEnvelopeEffectControlPointDuration; + const HalResult<std::vector<PwleV2OutputMapEntry>> frequencyToOutputAccelerationMap; void logFailures() const { logFailure<Capabilities>(capabilities, "getCapabilities"); @@ -284,6 +286,8 @@ public: "getMinEnvelopeEffectControlPointDuration"); logFailure<std::chrono::milliseconds>(maxEnvelopeEffectControlPointDuration, "getMaxEnvelopeEffectControlPointDuration"); + logFailure<std::vector<PwleV2OutputMapEntry>>(frequencyToOutputAccelerationMap, + "getfrequencyToOutputAccelerationMap"); } bool shouldRetry() const { @@ -296,7 +300,8 @@ public: qFactor.shouldRetry() || maxAmplitudes.shouldRetry() || maxEnvelopeEffectSize.shouldRetry() || minEnvelopeEffectControlPointDuration.shouldRetry() || - maxEnvelopeEffectControlPointDuration.shouldRetry(); + maxEnvelopeEffectControlPointDuration.shouldRetry() || + frequencyToOutputAccelerationMap.shouldRetry(); } private: @@ -327,7 +332,8 @@ public: mMaxAmplitudes, mMaxEnvelopeEffectSize, mMinEnvelopeEffectControlPointDuration, - mMaxEnvelopeEffectControlPointDuration}; + mMaxEnvelopeEffectControlPointDuration, + mFrequencyToOutputAccelerationMap}; } private: @@ -359,6 +365,8 @@ private: HalResult<std::chrono::milliseconds>::transactionFailed(MSG); HalResult<std::chrono::milliseconds> mMaxEnvelopeEffectControlPointDuration = HalResult<std::chrono::milliseconds>::transactionFailed(MSG); + HalResult<std::vector<Info::PwleV2OutputMapEntry>> mFrequencyToOutputAccelerationMap = + HalResult<std::vector<Info::PwleV2OutputMapEntry>>::transactionFailed(MSG); friend class HalWrapper; }; @@ -442,6 +450,8 @@ protected: virtual HalResult<int32_t> getMaxEnvelopeEffectSizeInternal(); virtual HalResult<std::chrono::milliseconds> getMinEnvelopeEffectControlPointDurationInternal(); virtual HalResult<std::chrono::milliseconds> getMaxEnvelopeEffectControlPointDurationInternal(); + virtual HalResult<std::vector<PwleV2OutputMapEntry>> + getFrequencyToOutputAccelerationMapInternal(); private: std::mutex mInfoMutex; @@ -518,12 +528,12 @@ protected: HalResult<float> getQFactorInternal() override final; HalResult<std::vector<float>> getMaxAmplitudesInternal() override final; HalResult<int32_t> getMaxEnvelopeEffectSizeInternal() override final; - HalResult<std::chrono::milliseconds> getMinEnvelopeEffectControlPointDurationInternal() override final; - HalResult<std::chrono::milliseconds> getMaxEnvelopeEffectControlPointDurationInternal() override final; + HalResult<std::vector<PwleV2OutputMapEntry>> getFrequencyToOutputAccelerationMapInternal() + override final; private: const reconnect_fn mReconnectFn; diff --git a/services/vibratorservice/test/VibratorHalWrapperAidlTest.cpp b/services/vibratorservice/test/VibratorHalWrapperAidlTest.cpp index 17f384d320..d42aa5684f 100644 --- a/services/vibratorservice/test/VibratorHalWrapperAidlTest.cpp +++ b/services/vibratorservice/test/VibratorHalWrapperAidlTest.cpp @@ -39,6 +39,7 @@ using aidl::android::hardware::vibrator::EffectStrength; using aidl::android::hardware::vibrator::IVibrator; using aidl::android::hardware::vibrator::IVibratorCallback; using aidl::android::hardware::vibrator::PrimitivePwle; +using aidl::android::hardware::vibrator::PwleV2OutputMapEntry; using aidl::android::hardware::vibrator::PwleV2Primitive; using aidl::android::hardware::vibrator::VendorEffect; using aidl::android::os::PersistableBundle; @@ -242,6 +243,11 @@ TEST_F(VibratorHalWrapperAidlTest, TestGetInfoDoesNotCacheFailedResult) { std::vector<CompositePrimitive> supportedPrimitives = {CompositePrimitive::CLICK}; std::vector<Braking> supportedBraking = {Braking::CLAB}; std::vector<float> amplitudes = {0.f, 1.f, 0.f}; + std::vector<PwleV2OutputMapEntry> + frequencyToOutputAccelerationMap{PwleV2OutputMapEntry(/*frequency=*/30.0f, + /*maxOutputAcceleration=*/0.2), + PwleV2OutputMapEntry(/*frequency=*/60.0f, + /*maxOutputAcceleration=*/0.8)}; std::vector<std::chrono::milliseconds> primitiveDurations; constexpr auto primitiveRange = ndk::enum_range<CompositePrimitive>(); @@ -323,6 +329,11 @@ TEST_F(VibratorHalWrapperAidlTest, TestGetInfoDoesNotCacheFailedResult) { .WillOnce(Return(ndk::ScopedAStatus::fromExceptionCode(EX_SECURITY))) .WillOnce(DoAll(SetArgPointee<0>(PWLE_V2_MIN_REQUIRED_PRIMITIVE_MAX_DURATION_MS), Return(ndk::ScopedAStatus::ok()))); + EXPECT_CALL(*mMockHal.get(), getPwleV2FrequencyToOutputAccelerationMap(_)) + .Times(Exactly(2)) + .WillOnce(Return(ndk::ScopedAStatus::fromExceptionCode(EX_SECURITY))) + .WillOnce(DoAll(SetArgPointee<0>(frequencyToOutputAccelerationMap), + Return(ndk::ScopedAStatus::ok()))); vibrator::Info failed = mWrapper->getInfo(); ASSERT_TRUE(failed.capabilities.isFailed()); @@ -342,6 +353,7 @@ TEST_F(VibratorHalWrapperAidlTest, TestGetInfoDoesNotCacheFailedResult) { ASSERT_TRUE(failed.maxEnvelopeEffectSize.isFailed()); ASSERT_TRUE(failed.minEnvelopeEffectControlPointDuration.isFailed()); ASSERT_TRUE(failed.maxEnvelopeEffectControlPointDuration.isFailed()); + ASSERT_TRUE(failed.frequencyToOutputAccelerationMap.isFailed()); vibrator::Info successful = mWrapper->getInfo(); ASSERT_EQ(vibrator::Capabilities::ON_CALLBACK, successful.capabilities.value()); @@ -364,6 +376,8 @@ TEST_F(VibratorHalWrapperAidlTest, TestGetInfoDoesNotCacheFailedResult) { successful.minEnvelopeEffectControlPointDuration.value()); ASSERT_EQ(std::chrono::milliseconds(PWLE_V2_MIN_REQUIRED_PRIMITIVE_MAX_DURATION_MS), successful.maxEnvelopeEffectControlPointDuration.value()); + ASSERT_EQ(frequencyToOutputAccelerationMap, + successful.frequencyToOutputAccelerationMap.value()); } TEST_F(VibratorHalWrapperAidlTest, TestGetInfoCachesResult) { @@ -377,6 +391,11 @@ TEST_F(VibratorHalWrapperAidlTest, TestGetInfoCachesResult) { constexpr int32_t PWLE_V2_MAX_ALLOWED_PRIMITIVE_MIN_DURATION_MS = 20; constexpr int32_t PWLE_V2_MIN_REQUIRED_PRIMITIVE_MAX_DURATION_MS = 1000; std::vector<Effect> supportedEffects = {Effect::CLICK, Effect::TICK}; + std::vector<PwleV2OutputMapEntry> + frequencyToOutputAccelerationMap{PwleV2OutputMapEntry(/*frequency=*/30.0f, + /*maxOutputAcceleration=*/0.2), + PwleV2OutputMapEntry(/*frequency=*/60.0f, + /*maxOutputAcceleration=*/0.8)}; EXPECT_CALL(*mMockHal.get(), getCapabilities(_)) .Times(Exactly(1)) @@ -432,6 +451,10 @@ TEST_F(VibratorHalWrapperAidlTest, TestGetInfoCachesResult) { .Times(Exactly(1)) .WillOnce(DoAll(SetArgPointee<0>(PWLE_V2_MIN_REQUIRED_PRIMITIVE_MAX_DURATION_MS), Return(ndk::ScopedAStatus::ok()))); + EXPECT_CALL(*mMockHal.get(), getPwleV2FrequencyToOutputAccelerationMap(_)) + .Times(Exactly(1)) + .WillOnce(DoAll(SetArgPointee<0>(frequencyToOutputAccelerationMap), + Return(ndk::ScopedAStatus::ok()))); std::vector<std::thread> threads; for (int i = 0; i < 10; i++) { @@ -460,6 +483,7 @@ TEST_F(VibratorHalWrapperAidlTest, TestGetInfoCachesResult) { info.minEnvelopeEffectControlPointDuration.value()); ASSERT_EQ(std::chrono::milliseconds(PWLE_V2_MIN_REQUIRED_PRIMITIVE_MAX_DURATION_MS), info.maxEnvelopeEffectControlPointDuration.value()); + ASSERT_EQ(frequencyToOutputAccelerationMap, info.frequencyToOutputAccelerationMap.value()); } TEST_F(VibratorHalWrapperAidlTest, TestPerformEffectWithCallbackSupport) { diff --git a/services/vibratorservice/test/VibratorHalWrapperHidlV1_0Test.cpp b/services/vibratorservice/test/VibratorHalWrapperHidlV1_0Test.cpp index a09ddecf91..d6dab8d13c 100644 --- a/services/vibratorservice/test/VibratorHalWrapperHidlV1_0Test.cpp +++ b/services/vibratorservice/test/VibratorHalWrapperHidlV1_0Test.cpp @@ -223,6 +223,7 @@ TEST_F(VibratorHalWrapperHidlV1_0Test, TestGetInfoDoesNotCacheFailedResult) { ASSERT_TRUE(info.maxEnvelopeEffectSize.isUnsupported()); ASSERT_TRUE(info.minEnvelopeEffectControlPointDuration.isUnsupported()); ASSERT_TRUE(info.maxEnvelopeEffectControlPointDuration.isUnsupported()); + ASSERT_TRUE(info.frequencyToOutputAccelerationMap.isUnsupported()); } TEST_F(VibratorHalWrapperHidlV1_0Test, TestGetInfoWithoutAmplitudeControl) { @@ -259,6 +260,7 @@ TEST_F(VibratorHalWrapperHidlV1_0Test, TestGetInfoCachesResult) { ASSERT_TRUE(info.maxEnvelopeEffectSize.isUnsupported()); ASSERT_TRUE(info.minEnvelopeEffectControlPointDuration.isUnsupported()); ASSERT_TRUE(info.maxEnvelopeEffectControlPointDuration.isUnsupported()); + ASSERT_TRUE(info.frequencyToOutputAccelerationMap.isUnsupported()); } TEST_F(VibratorHalWrapperHidlV1_0Test, TestPerformEffect) { diff --git a/services/vibratorservice/test/VibratorManagerHalWrapperAidlTest.cpp b/services/vibratorservice/test/VibratorManagerHalWrapperAidlTest.cpp index 764d9bea4f..ca13c0b70e 100644 --- a/services/vibratorservice/test/VibratorManagerHalWrapperAidlTest.cpp +++ b/services/vibratorservice/test/VibratorManagerHalWrapperAidlTest.cpp @@ -31,10 +31,12 @@ using aidl::android::hardware::vibrator::CompositeEffect; using aidl::android::hardware::vibrator::CompositePrimitive; using aidl::android::hardware::vibrator::Effect; using aidl::android::hardware::vibrator::EffectStrength; +using aidl::android::hardware::vibrator::IVibrationSession; using aidl::android::hardware::vibrator::IVibrator; using aidl::android::hardware::vibrator::IVibratorCallback; using aidl::android::hardware::vibrator::IVibratorManager; using aidl::android::hardware::vibrator::PrimitivePwle; +using aidl::android::hardware::vibrator::VibrationSessionConfig; using namespace android; using namespace testing; @@ -55,6 +57,12 @@ public: MOCK_METHOD(ndk::ScopedAStatus, triggerSynced, (const std::shared_ptr<IVibratorCallback>& cb), (override)); MOCK_METHOD(ndk::ScopedAStatus, cancelSynced, (), (override)); + MOCK_METHOD(ndk::ScopedAStatus, startSession, + (const std::vector<int32_t>& ids, const VibrationSessionConfig& s, + const std::shared_ptr<IVibratorCallback>& cb, + std::shared_ptr<IVibrationSession>* ret), + (override)); + MOCK_METHOD(ndk::ScopedAStatus, clearSessions, (), (override)); MOCK_METHOD(ndk::ScopedAStatus, getInterfaceVersion, (int32_t*), (override)); MOCK_METHOD(ndk::ScopedAStatus, getInterfaceHash, (std::string*), (override)); MOCK_METHOD(ndk::SpAIBinder, asBinder, (), (override)); |