diff options
46 files changed, 1094 insertions, 499 deletions
diff --git a/cmds/installd/InstalldNativeService.cpp b/cmds/installd/InstalldNativeService.cpp index 1d7dd5f51e..6ee3070306 100644 --- a/cmds/installd/InstalldNativeService.cpp +++ b/cmds/installd/InstalldNativeService.cpp @@ -230,6 +230,19 @@ binder::Status checkArgumentPath(const std::optional<std::string>& path) { } } +binder::Status checkArgumentFileName(const std::string& path) { + if (path.empty()) { + return exception(binder::Status::EX_ILLEGAL_ARGUMENT, "Missing name"); + } + for (const char& c : path) { + if (c == '\0' || c == '\n' || c == '/') { + return exception(binder::Status::EX_ILLEGAL_ARGUMENT, + StringPrintf("Name %s is malformed", path.c_str())); + } + } + return ok(); +} + #define ENFORCE_UID(uid) { \ binder::Status status = checkUid((uid)); \ if (!status.isOk()) { \ @@ -266,6 +279,14 @@ binder::Status checkArgumentPath(const std::optional<std::string>& path) { } \ } +#define CHECK_ARGUMENT_FILE_NAME(path) \ + { \ + binder::Status status = checkArgumentFileName((path)); \ + if (!status.isOk()) { \ + return status; \ + } \ + } + #ifdef GRANULAR_LOCKS /** @@ -1006,6 +1027,12 @@ binder::Status InstalldNativeService::clearAppProfiles(const std::string& packag const std::string& profileName) { ENFORCE_UID(AID_SYSTEM); CHECK_ARGUMENT_PACKAGE_NAME(packageName); + CHECK_ARGUMENT_FILE_NAME(profileName); + if (!base::EndsWith(profileName, ".prof")) { + return exception(binder::Status::EX_ILLEGAL_ARGUMENT, + StringPrintf("Profile name %s does not end with .prof", + profileName.c_str())); + } LOCK_PACKAGE(); binder::Status res = ok(); @@ -3025,7 +3052,19 @@ binder::Status InstalldNativeService::copySystemProfile(const std::string& syste int32_t packageUid, const std::string& packageName, const std::string& profileName, bool* _aidl_return) { ENFORCE_UID(AID_SYSTEM); + CHECK_ARGUMENT_PATH(systemProfile); + if (!base::EndsWith(systemProfile, ".prof")) { + return exception(binder::Status::EX_ILLEGAL_ARGUMENT, + StringPrintf("System profile path %s does not end with .prof", + systemProfile.c_str())); + } CHECK_ARGUMENT_PACKAGE_NAME(packageName); + CHECK_ARGUMENT_FILE_NAME(profileName); + if (!base::EndsWith(profileName, ".prof")) { + return exception(binder::Status::EX_ILLEGAL_ARGUMENT, + StringPrintf("Profile name %s does not end with .prof", + profileName.c_str())); + } LOCK_PACKAGE(); *_aidl_return = copy_system_profile(systemProfile, packageUid, packageName, profileName); return ok(); diff --git a/cmds/installd/tests/installd_dexopt_test.cpp b/cmds/installd/tests/installd_dexopt_test.cpp index 3849c40ac2..6ef41e3258 100644 --- a/cmds/installd/tests/installd_dexopt_test.cpp +++ b/cmds/installd/tests/installd_dexopt_test.cpp @@ -1371,6 +1371,58 @@ TEST_F(ProfileTest, ProfilePrepareFailProfileChangedUid) { /*has_user_id*/ true, /*expected_result*/ false); } +TEST_F(ProfileTest, ClearAppProfilesOk) { + LOG(INFO) << "ClearAppProfilesOk"; + + ASSERT_BINDER_SUCCESS(service_->clearAppProfiles(package_name_, "primary.prof")); + ASSERT_BINDER_SUCCESS(service_->clearAppProfiles(package_name_, "image_editor.split.prof")); +} + +TEST_F(ProfileTest, ClearAppProfilesFailWrongProfileName) { + LOG(INFO) << "ClearAppProfilesFailWrongProfileName"; + + ASSERT_BINDER_FAIL( + service_->clearAppProfiles(package_name_, + "../../../../dalvik-cache/arm64/" + "system@app@SecureElement@SecureElement.apk@classes.vdex")); + ASSERT_BINDER_FAIL(service_->clearAppProfiles(package_name_, "image_editor.split.apk")); +} + +TEST_F(ProfileTest, CopySystemProfileOk) { + LOG(INFO) << "CopySystemProfileOk"; + + bool result; + ASSERT_BINDER_SUCCESS( + service_->copySystemProfile("/data/app/random.string/package.name.random/base.apk.prof", + kTestAppUid, package_name_, "primary.prof", &result)); +} + +TEST_F(ProfileTest, CopySystemProfileFailWrongSystemProfilePath) { + LOG(INFO) << "CopySystemProfileFailWrongSystemProfilePath"; + + bool result; + ASSERT_BINDER_FAIL(service_->copySystemProfile("../../secret.dat", kTestAppUid, package_name_, + "primary.prof", &result)); + ASSERT_BINDER_FAIL(service_->copySystemProfile("/data/user/package.name/secret.data", + kTestAppUid, package_name_, "primary.prof", + &result)); +} + +TEST_F(ProfileTest, CopySystemProfileFailWrongProfileName) { + LOG(INFO) << "CopySystemProfileFailWrongProfileName"; + + bool result; + ASSERT_BINDER_FAIL( + service_->copySystemProfile("/data/app/random.string/package.name.random/base.apk.prof", + kTestAppUid, package_name_, + "../../../../dalvik-cache/arm64/test.vdex", &result)); + ASSERT_BINDER_FAIL( + service_->copySystemProfile("/data/app/random.string/package.name.random/base.apk.prof", + kTestAppUid, package_name_, "/test.prof", &result)); + ASSERT_BINDER_FAIL( + service_->copySystemProfile("/data/app/random.string/package.name.random/base.apk.prof", + kTestAppUid, package_name_, "base.apk", &result)); +} class BootProfileTest : public ProfileTest { public: diff --git a/libs/binder/Android.bp b/libs/binder/Android.bp index 5e9f5407fd..6f505b44fb 100644 --- a/libs/binder/Android.bp +++ b/libs/binder/Android.bp @@ -335,6 +335,34 @@ cc_library_shared { defaults: ["libbinder_tls_defaults"], } +cc_library_shared { + name: "libbinder_trusty", + vendor: true, + srcs: [ + "RpcTransportTipcAndroid.cpp", + "RpcTrusty.cpp", + ], + + shared_libs: [ + "libbinder", + "liblog", + "libtrusty", + "libutils", + ], + static_libs: [ + "libbase", + ], + export_include_dirs: ["include_trusty"], + + // Most of Android doesn't need this library and shouldn't use it, + // so we restrict its visibility to the Trusty-specific packages. + visibility: [ + ":__subpackages__", + "//system/core/trusty:__subpackages__", + "//vendor:__subpackages__", + ], +} + // For testing cc_library_static { name: "libbinder_tls_static", diff --git a/libs/binder/RpcTransportTipcAndroid.cpp b/libs/binder/RpcTransportTipcAndroid.cpp new file mode 100644 index 0000000000..79983f46d3 --- /dev/null +++ b/libs/binder/RpcTransportTipcAndroid.cpp @@ -0,0 +1,218 @@ +/* + * Copyright (C) 2022 The Android Open Source Project + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +#define LOG_TAG "RpcTransportTipcAndroid" + +#include <binder/RpcSession.h> +#include <binder/RpcTransportTipcAndroid.h> +#include <log/log.h> +#include <poll.h> +#include <trusty/tipc.h> + +#include "FdTrigger.h" +#include "RpcState.h" +#include "RpcTransportUtils.h" + +using android::base::Error; +using android::base::Result; + +namespace android { + +namespace { + +// RpcTransport for writing Trusty IPC clients in Android. +class RpcTransportTipcAndroid : public RpcTransport { +public: + explicit RpcTransportTipcAndroid(android::base::unique_fd socket) + : mSocket(std::move(socket)) {} + + status_t pollRead() override { + if (mReadBufferPos < mReadBufferSize) { + // We have more data in the read buffer + return OK; + } + + // Trusty IPC device is not a socket, so MSG_PEEK is not available + pollfd pfd{.fd = mSocket.get(), .events = static_cast<int16_t>(POLLIN), .revents = 0}; + ssize_t ret = TEMP_FAILURE_RETRY(::poll(&pfd, 1, 0)); + if (ret < 0) { + int savedErrno = errno; + if (savedErrno == EAGAIN || savedErrno == EWOULDBLOCK) { + return WOULD_BLOCK; + } + + LOG_RPC_DETAIL("RpcTransport poll(): %s", strerror(savedErrno)); + return -savedErrno; + } + + if (pfd.revents & POLLNVAL) { + return BAD_VALUE; + } + if (pfd.revents & POLLERR) { + return DEAD_OBJECT; + } + if (pfd.revents & POLLHUP) { + return DEAD_OBJECT; + } + if (pfd.revents & POLLIN) { + return OK; + } + + return WOULD_BLOCK; + } + + status_t interruptableWriteFully( + FdTrigger* fdTrigger, iovec* iovs, int niovs, + const std::optional<android::base::function_ref<status_t()>>& altPoll, + const std::vector<std::variant<base::unique_fd, base::borrowed_fd>>* ancillaryFds) + override { + auto writeFn = [&](iovec* iovs, size_t niovs) -> ssize_t { + // TODO: send ancillaryFds. For now, we just abort if anyone tries + // to send any. + LOG_ALWAYS_FATAL_IF(ancillaryFds != nullptr && !ancillaryFds->empty(), + "File descriptors are not supported on Trusty yet"); + return TEMP_FAILURE_RETRY(tipc_send(mSocket.get(), iovs, niovs, nullptr, 0)); + }; + return interruptableReadOrWrite(mSocket.get(), fdTrigger, iovs, niovs, writeFn, "tipc_send", + POLLOUT, altPoll); + } + + status_t interruptableReadFully( + FdTrigger* fdTrigger, iovec* iovs, int niovs, + const std::optional<android::base::function_ref<status_t()>>& altPoll, + std::vector<std::variant<base::unique_fd, base::borrowed_fd>>* /*ancillaryFds*/) + override { + auto readFn = [&](iovec* iovs, size_t niovs) -> ssize_t { + // Fill the read buffer at most once per readFn call, then try to + // return as much of it as possible. If the input iovecs are spread + // across multiple messages that require multiple fillReadBuffer + // calls, we expect the caller to advance the iovecs past the first + // read and call readFn as many times as needed to get all the data + status_t ret = fillReadBuffer(); + if (ret != OK) { + return ret; + } + + ssize_t processSize = 0; + for (size_t i = 0; i < niovs && mReadBufferPos < mReadBufferSize; i++) { + auto& iov = iovs[i]; + size_t numBytes = std::min(iov.iov_len, mReadBufferSize - mReadBufferPos); + memcpy(iov.iov_base, mReadBuffer.get() + mReadBufferPos, numBytes); + mReadBufferPos += numBytes; + processSize += numBytes; + } + + return processSize; + }; + return interruptableReadOrWrite(mSocket.get(), fdTrigger, iovs, niovs, readFn, "read", + POLLIN, altPoll); + } + +private: + status_t fillReadBuffer() { + if (mReadBufferPos < mReadBufferSize) { + return OK; + } + + if (!mReadBuffer) { + // Guarantee at least kDefaultBufferSize bytes + mReadBufferCapacity = std::max(mReadBufferCapacity, kDefaultBufferSize); + mReadBuffer.reset(new (std::nothrow) uint8_t[mReadBufferCapacity]); + if (!mReadBuffer) { + return NO_MEMORY; + } + } + + // Reset the size and position in case we have to exit with an error. + // After we read a message into the buffer, we update the size + // with the actual value. + mReadBufferPos = 0; + mReadBufferSize = 0; + + while (true) { + ssize_t processSize = + TEMP_FAILURE_RETRY(read(mSocket.get(), mReadBuffer.get(), mReadBufferCapacity)); + if (processSize == 0) { + return DEAD_OBJECT; + } else if (processSize < 0) { + int savedErrno = errno; + if (savedErrno == EMSGSIZE) { + // Buffer was too small, double it and retry + if (__builtin_mul_overflow(mReadBufferCapacity, 2, &mReadBufferCapacity)) { + return NO_MEMORY; + } + mReadBuffer.reset(new (std::nothrow) uint8_t[mReadBufferCapacity]); + if (!mReadBuffer) { + return NO_MEMORY; + } + continue; + } else { + LOG_RPC_DETAIL("RpcTransport fillBuffer(): %s", strerror(savedErrno)); + return -savedErrno; + } + } else { + mReadBufferSize = static_cast<size_t>(processSize); + return OK; + } + } + } + + base::unique_fd mSocket; + + // For now, we copy all the input data into a temporary buffer because + // we might get multiple interruptableReadFully calls per message, but + // the tipc device only allows one read call. We read every message into + // this temporary buffer, then return pieces of it from our method. + // + // The special transaction GET_MAX_THREADS takes 40 bytes, so the default + // size should start pretty high. + static constexpr size_t kDefaultBufferSize = 64; + std::unique_ptr<uint8_t[]> mReadBuffer; + size_t mReadBufferPos = 0; + size_t mReadBufferSize = 0; + size_t mReadBufferCapacity = 0; +}; + +// RpcTransportCtx for Trusty. +class RpcTransportCtxTipcAndroid : public RpcTransportCtx { +public: + std::unique_ptr<RpcTransport> newTransport(android::base::unique_fd fd, + FdTrigger*) const override { + return std::make_unique<RpcTransportTipcAndroid>(std::move(fd)); + } + std::vector<uint8_t> getCertificate(RpcCertificateFormat) const override { return {}; } +}; + +} // namespace + +std::unique_ptr<RpcTransportCtx> RpcTransportCtxFactoryTipcAndroid::newServerCtx() const { + return std::make_unique<RpcTransportCtxTipcAndroid>(); +} + +std::unique_ptr<RpcTransportCtx> RpcTransportCtxFactoryTipcAndroid::newClientCtx() const { + return std::make_unique<RpcTransportCtxTipcAndroid>(); +} + +const char* RpcTransportCtxFactoryTipcAndroid::toCString() const { + return "trusty"; +} + +std::unique_ptr<RpcTransportCtxFactory> RpcTransportCtxFactoryTipcAndroid::make() { + return std::unique_ptr<RpcTransportCtxFactoryTipcAndroid>( + new RpcTransportCtxFactoryTipcAndroid()); +} + +} // namespace android diff --git a/libs/binder/RpcTrusty.cpp b/libs/binder/RpcTrusty.cpp new file mode 100644 index 0000000000..ea49eef676 --- /dev/null +++ b/libs/binder/RpcTrusty.cpp @@ -0,0 +1,46 @@ +/* + * Copyright (C) 2022 The Android Open Source Project + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +#define LOG_TAG "RpcTrusty" + +#include <android-base/logging.h> +#include <android-base/unique_fd.h> +#include <binder/RpcSession.h> +#include <binder/RpcTransportTipcAndroid.h> +#include <trusty/tipc.h> + +namespace android { + +using android::base::unique_fd; + +sp<IBinder> RpcTrustyConnect(const char* device, const char* port) { + auto session = RpcSession::make(RpcTransportCtxFactoryTipcAndroid::make()); + auto request = [=] { + int tipcFd = tipc_connect(device, port); + if (tipcFd < 0) { + LOG(ERROR) << "Failed to connect to Trusty service. Error code: " << tipcFd; + return unique_fd(); + } + return unique_fd(tipcFd); + }; + if (status_t status = session->setupPreconnectedClient(unique_fd{}, request); status != OK) { + LOG(ERROR) << "Failed to set up Trusty client. Error: " << statusToString(status).c_str(); + return nullptr; + } + return session->getRootObject(); +} + +} // namespace android diff --git a/libs/binder/include_trusty/binder/RpcTransportTipcAndroid.h b/libs/binder/include_trusty/binder/RpcTransportTipcAndroid.h new file mode 100644 index 0000000000..4a4172a3a7 --- /dev/null +++ b/libs/binder/include_trusty/binder/RpcTransportTipcAndroid.h @@ -0,0 +1,40 @@ +/* + * Copyright (C) 2022 The Android Open Source Project + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +// Wraps the transport layer of RPC. Implementation uses Trusty IPC. + +#pragma once + +#include <memory> + +#include <binder/RpcTransport.h> + +namespace android { + +// RpcTransportCtxFactory for writing Trusty IPC clients in Android. +class RpcTransportCtxFactoryTipcAndroid : public RpcTransportCtxFactory { +public: + static std::unique_ptr<RpcTransportCtxFactory> make(); + + std::unique_ptr<RpcTransportCtx> newServerCtx() const override; + std::unique_ptr<RpcTransportCtx> newClientCtx() const override; + const char* toCString() const override; + +private: + RpcTransportCtxFactoryTipcAndroid() = default; +}; + +} // namespace android diff --git a/libs/binder/include_trusty/binder/RpcTrusty.h b/libs/binder/include_trusty/binder/RpcTrusty.h new file mode 100644 index 0000000000..f124e0c824 --- /dev/null +++ b/libs/binder/include_trusty/binder/RpcTrusty.h @@ -0,0 +1,25 @@ +/* + * Copyright (C) 2022 The Android Open Source Project + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +#pragma once + +#include <binder/IBinder.h> + +namespace android { + +sp<IBinder> RpcTrustyConnect(const char* device, const char* port); + +} // namespace android diff --git a/libs/binder/tests/binderAllocationLimits.cpp b/libs/binder/tests/binderAllocationLimits.cpp index 60b3c94f5e..a2ab8ab302 100644 --- a/libs/binder/tests/binderAllocationLimits.cpp +++ b/libs/binder/tests/binderAllocationLimits.cpp @@ -27,6 +27,8 @@ #include <functional> #include <vector> +static android::String8 gEmpty(""); // make sure first allocation from optimization runs + struct DestructionAction { DestructionAction(std::function<void()> f) : mF(std::move(f)) {} ~DestructionAction() { mF(); }; diff --git a/services/inputflinger/UnwantedInteractionBlocker.cpp b/services/inputflinger/UnwantedInteractionBlocker.cpp index 4e0f0c3bf5..ec41025e9d 100644 --- a/services/inputflinger/UnwantedInteractionBlocker.cpp +++ b/services/inputflinger/UnwantedInteractionBlocker.cpp @@ -77,8 +77,7 @@ static std::string toLower(std::string s) { } static bool isFromTouchscreen(int32_t source) { - return isFromSource(source, AINPUT_SOURCE_TOUCHSCREEN) && - !isFromSource(source, AINPUT_SOURCE_STYLUS); + return isFromSource(source, AINPUT_SOURCE_TOUCHSCREEN); } static ::base::TimeTicks toChromeTimestamp(nsecs_t eventTime) { @@ -99,17 +98,15 @@ static bool isPalmRejectionEnabled() { return false; } -static int getLinuxToolType(int32_t toolType) { - switch (toolType) { - case AMOTION_EVENT_TOOL_TYPE_FINGER: - return MT_TOOL_FINGER; - case AMOTION_EVENT_TOOL_TYPE_STYLUS: - return MT_TOOL_PEN; - case AMOTION_EVENT_TOOL_TYPE_PALM: - return MT_TOOL_PALM; +static int getLinuxToolCode(int toolType) { + if (toolType == AMOTION_EVENT_TOOL_TYPE_STYLUS) { + return BTN_TOOL_PEN; } - ALOGW("Got tool type %" PRId32 ", converting to MT_TOOL_FINGER", toolType); - return MT_TOOL_FINGER; + if (toolType == AMOTION_EVENT_TOOL_TYPE_FINGER) { + return BTN_TOOL_FINGER; + } + ALOGW("Got tool type %" PRId32 ", converting to BTN_TOOL_FINGER", toolType); + return BTN_TOOL_FINGER; } static int32_t getActionUpForPointerId(const NotifyMotionArgs& args, int32_t pointerId) { @@ -144,23 +141,6 @@ static int32_t resolveActionForPointer(uint8_t pointerIndex, int32_t action) { return AMOTION_EVENT_ACTION_MOVE; } -/** - * Remove the data for the provided pointers from the args. The pointers are identified by their - * pointerId, not by the index inside the array. - * Return the new NotifyMotionArgs struct that has the remaining pointers. - * The only fields that may be different in the returned args from the provided args are: - * - action - * - pointerCount - * - pointerProperties - * - pointerCoords - * Action might change because it contains a pointer index. If another pointer is removed, the - * active pointer index would be shifted. - * Do not call this function for events with POINTER_UP or POINTER_DOWN events when removed pointer - * id is the acting pointer id. - * - * @param args the args from which the pointers should be removed - * @param pointerIds the pointer ids of the pointers that should be removed - */ NotifyMotionArgs removePointerIds(const NotifyMotionArgs& args, const std::set<int32_t>& pointerIds) { const uint8_t actionIndex = MotionEvent::getActionIndex(args.action); @@ -206,6 +186,26 @@ NotifyMotionArgs removePointerIds(const NotifyMotionArgs& args, return newArgs; } +/** + * Remove stylus pointers from the provided NotifyMotionArgs. + * + * Return NotifyMotionArgs where the stylus pointers have been removed. + * If this results in removal of the active pointer, then return nullopt. + */ +static std::optional<NotifyMotionArgs> removeStylusPointerIds(const NotifyMotionArgs& args) { + std::set<int32_t> stylusPointerIds; + for (uint32_t i = 0; i < args.pointerCount; i++) { + if (args.pointerProperties[i].toolType == AMOTION_EVENT_TOOL_TYPE_STYLUS) { + stylusPointerIds.insert(args.pointerProperties[i].id); + } + } + NotifyMotionArgs withoutStylusPointers = removePointerIds(args, stylusPointerIds); + if (withoutStylusPointers.pointerCount == 0 || withoutStylusPointers.action == ACTION_UNKNOWN) { + return std::nullopt; + } + return withoutStylusPointers; +} + std::optional<AndroidPalmFilterDeviceInfo> createPalmFilterDeviceInfo( const InputDeviceInfo& deviceInfo) { if (!isFromTouchscreen(deviceInfo.getSources())) { @@ -562,7 +562,7 @@ std::vector<::ui::InProgressTouchEvdev> getTouches(const NotifyMotionArgs& args, touches.emplace_back(::ui::InProgressTouchEvdev()); touches.back().major = args.pointerCoords[i].getAxisValue(AMOTION_EVENT_AXIS_TOUCH_MAJOR); touches.back().minor = args.pointerCoords[i].getAxisValue(AMOTION_EVENT_AXIS_TOUCH_MINOR); - touches.back().tool_type = getLinuxToolType(args.pointerProperties[i].toolType); + // The field 'tool_type' is not used for palm rejection // Whether there is new information for the touch. touches.back().altered = true; @@ -609,32 +609,16 @@ std::vector<::ui::InProgressTouchEvdev> getTouches(const NotifyMotionArgs& args, // The fields 'radius_x' and 'radius_x' are not used for palm rejection touches.back().pressure = args.pointerCoords[i].getAxisValue(AMOTION_EVENT_AXIS_PRESSURE); - touches.back().tool_code = BTN_TOOL_FINGER; + touches.back().tool_code = getLinuxToolCode(args.pointerProperties[i].toolType); // The field 'orientation' is not used for palm rejection // The fields 'tilt_x' and 'tilt_y' are not used for palm rejection - touches.back().reported_tool_type = ::ui::EventPointerType::kTouch; + // The field 'reported_tool_type' is not used for palm rejection touches.back().stylus_button = false; } return touches; } -std::vector<NotifyMotionArgs> PalmRejector::processMotion(const NotifyMotionArgs& args) { - if (mPalmDetectionFilter == nullptr) { - return {args}; - } - const bool skipThisEvent = args.action == AMOTION_EVENT_ACTION_HOVER_ENTER || - args.action == AMOTION_EVENT_ACTION_HOVER_MOVE || - args.action == AMOTION_EVENT_ACTION_HOVER_EXIT || - args.action == AMOTION_EVENT_ACTION_BUTTON_PRESS || - args.action == AMOTION_EVENT_ACTION_BUTTON_RELEASE || - args.action == AMOTION_EVENT_ACTION_SCROLL; - if (skipThisEvent) { - // Lets not process hover events, button events, or scroll for now. - return {args}; - } - if (args.action == AMOTION_EVENT_ACTION_DOWN) { - mSuppressedPointerIds.clear(); - } +std::set<int32_t> PalmRejector::detectPalmPointers(const NotifyMotionArgs& args) { std::bitset<::ui::kNumTouchEvdevSlots> slotsToHold; std::bitset<::ui::kNumTouchEvdevSlots> slotsToSuppress; @@ -642,6 +626,7 @@ std::vector<NotifyMotionArgs> PalmRejector::processMotion(const NotifyMotionArgs // the slots that have been removed due to the incoming event. SlotState oldSlotState = mSlotState; mSlotState.update(args); + std::vector<::ui::InProgressTouchEvdev> touches = getTouches(args, mDeviceInfo, oldSlotState, mSlotState); ::base::TimeTicks chromeTimestamp = toChromeTimestamp(args.eventTime); @@ -653,14 +638,14 @@ std::vector<NotifyMotionArgs> PalmRejector::processMotion(const NotifyMotionArgs } ALOGD("Filter: touches = %s", touchesStream.str().c_str()); } + mPalmDetectionFilter->Filter(touches, chromeTimestamp, &slotsToHold, &slotsToSuppress); ALOGD_IF(DEBUG_MODEL, "Response: slotsToHold = %s, slotsToSuppress = %s", slotsToHold.to_string().c_str(), slotsToSuppress.to_string().c_str()); // Now that we know which slots should be suppressed, let's convert those to pointer id's. - std::set<int32_t> oldSuppressedIds; - std::swap(oldSuppressedIds, mSuppressedPointerIds); + std::set<int32_t> newSuppressedIds; for (size_t i = 0; i < args.pointerCount; i++) { const int32_t pointerId = args.pointerProperties[i].id; std::optional<size_t> slot = oldSlotState.getSlotForPointerId(pointerId); @@ -669,9 +654,41 @@ std::vector<NotifyMotionArgs> PalmRejector::processMotion(const NotifyMotionArgs LOG_ALWAYS_FATAL_IF(!slot, "Could not find slot for pointer id %" PRId32, pointerId); } if (slotsToSuppress.test(*slot)) { - mSuppressedPointerIds.insert(pointerId); + newSuppressedIds.insert(pointerId); } } + return newSuppressedIds; +} + +std::vector<NotifyMotionArgs> PalmRejector::processMotion(const NotifyMotionArgs& args) { + if (mPalmDetectionFilter == nullptr) { + return {args}; + } + const bool skipThisEvent = args.action == AMOTION_EVENT_ACTION_HOVER_ENTER || + args.action == AMOTION_EVENT_ACTION_HOVER_MOVE || + args.action == AMOTION_EVENT_ACTION_HOVER_EXIT || + args.action == AMOTION_EVENT_ACTION_BUTTON_PRESS || + args.action == AMOTION_EVENT_ACTION_BUTTON_RELEASE || + args.action == AMOTION_EVENT_ACTION_SCROLL; + if (skipThisEvent) { + // Lets not process hover events, button events, or scroll for now. + return {args}; + } + if (args.action == AMOTION_EVENT_ACTION_DOWN) { + mSuppressedPointerIds.clear(); + } + + std::set<int32_t> oldSuppressedIds; + std::swap(oldSuppressedIds, mSuppressedPointerIds); + + std::optional<NotifyMotionArgs> touchOnlyArgs = removeStylusPointerIds(args); + if (touchOnlyArgs) { + mSuppressedPointerIds = detectPalmPointers(*touchOnlyArgs); + } else { + // This is a stylus-only event. + // We can skip this event and just keep the suppressed pointer ids the same as before. + mSuppressedPointerIds = oldSuppressedIds; + } std::vector<NotifyMotionArgs> argsWithoutUnwantedPointers = cancelSuppressedPointers(args, oldSuppressedIds, mSuppressedPointerIds); @@ -691,7 +708,7 @@ std::vector<NotifyMotionArgs> PalmRejector::processMotion(const NotifyMotionArgs return argsWithoutUnwantedPointers; } -const AndroidPalmFilterDeviceInfo& PalmRejector::getPalmFilterDeviceInfo() { +const AndroidPalmFilterDeviceInfo& PalmRejector::getPalmFilterDeviceInfo() const { return mDeviceInfo; } diff --git a/services/inputflinger/UnwantedInteractionBlocker.h b/services/inputflinger/UnwantedInteractionBlocker.h index 8ff965851f..5d0dde8e64 100644 --- a/services/inputflinger/UnwantedInteractionBlocker.h +++ b/services/inputflinger/UnwantedInteractionBlocker.h @@ -43,6 +43,25 @@ std::optional<AndroidPalmFilterDeviceInfo> createPalmFilterDeviceInfo( static constexpr int32_t ACTION_UNKNOWN = -1; +/** + * Remove the data for the provided pointers from the args. The pointers are identified by their + * pointerId, not by the index inside the array. + * Return the new NotifyMotionArgs struct that has the remaining pointers. + * The only fields that may be different in the returned args from the provided args are: + * - action + * - pointerCount + * - pointerProperties + * - pointerCoords + * Action might change because it contains a pointer index. If another pointer is removed, the + * active pointer index would be shifted. + * + * If the active pointer id is removed (for example, for events like + * POINTER_UP or POINTER_DOWN), then the action is set to ACTION_UNKNOWN. It is up to the caller + * to set the action appropriately after the call. + * + * @param args the args from which the pointers should be removed + * @param pointerIds the pointer ids of the pointers that should be removed + */ NotifyMotionArgs removePointerIds(const NotifyMotionArgs& args, const std::set<int32_t>& pointerIds); @@ -147,13 +166,21 @@ public: std::vector<NotifyMotionArgs> processMotion(const NotifyMotionArgs& args); // Get the device info of this device, for comparison purposes - const AndroidPalmFilterDeviceInfo& getPalmFilterDeviceInfo(); + const AndroidPalmFilterDeviceInfo& getPalmFilterDeviceInfo() const; std::string dump() const; private: PalmRejector(const PalmRejector&) = delete; PalmRejector& operator=(const PalmRejector&) = delete; + /** + * Update the slot state and send this event to the palm rejection model for palm detection. + * Return the pointer ids that should be suppressed. + * + * This function is not const because it has side-effects. It will update the slot state using + * the incoming args! Also, it will call Filter(..), which has side-effects. + */ + std::set<int32_t> detectPalmPointers(const NotifyMotionArgs& args); std::unique_ptr<::ui::SharedPalmDetectionFilterState> mSharedPalmState; AndroidPalmFilterDeviceInfo mDeviceInfo; std::unique_ptr<::ui::PalmDetectionFilter> mPalmDetectionFilter; diff --git a/services/inputflinger/reader/InputDevice.cpp b/services/inputflinger/reader/InputDevice.cpp index b67777fc0a..5c9e63e80e 100644 --- a/services/inputflinger/reader/InputDevice.cpp +++ b/services/inputflinger/reader/InputDevice.cpp @@ -110,7 +110,8 @@ void InputDevice::dump(std::string& dump, const std::string& eventHubDevStr) { dump += "<none>\n"; } dump += StringPrintf(INDENT2 "HasMic: %s\n", toString(mHasMic)); - dump += StringPrintf(INDENT2 "Sources: 0x%08x\n", deviceInfo.getSources()); + dump += StringPrintf(INDENT2 "Sources: %s\n", + inputEventSourceToString(deviceInfo.getSources()).c_str()); dump += StringPrintf(INDENT2 "KeyboardType: %d\n", deviceInfo.getKeyboardType()); dump += StringPrintf(INDENT2 "ControllerNum: %d\n", deviceInfo.getControllerNumber()); @@ -128,10 +129,10 @@ void InputDevice::dump(std::string& dump, const std::string& eventHubDevStr) { snprintf(name, sizeof(name), "%d", range.axis); } dump += StringPrintf(INDENT3 - "%s: source=0x%08x, " + "%s: source=%s, " "min=%0.3f, max=%0.3f, flat=%0.3f, fuzz=%0.3f, resolution=%0.3f\n", - name, range.source, range.min, range.max, range.flat, range.fuzz, - range.resolution); + name, inputEventSourceToString(range.source).c_str(), range.min, + range.max, range.flat, range.fuzz, range.resolution); } } diff --git a/services/inputflinger/reader/InputReader.cpp b/services/inputflinger/reader/InputReader.cpp index 11a2dbeb3b..dc410513e1 100644 --- a/services/inputflinger/reader/InputReader.cpp +++ b/services/inputflinger/reader/InputReader.cpp @@ -216,9 +216,9 @@ void InputReader::addDeviceLocked(nsecs_t when, int32_t eventHubId) { "(ignored non-input device)", device->getId(), eventHubId, identifier.name.c_str(), identifier.descriptor.c_str()); } else { - ALOGI("Device added: id=%d, eventHubId=%d, name='%s', descriptor='%s',sources=0x%08x", + ALOGI("Device added: id=%d, eventHubId=%d, name='%s', descriptor='%s',sources=%s", device->getId(), eventHubId, identifier.name.c_str(), identifier.descriptor.c_str(), - device->getSources()); + inputEventSourceToString(device->getSources()).c_str()); } mDevices.emplace(eventHubId, device); @@ -270,9 +270,10 @@ void InputReader::removeDeviceLocked(nsecs_t when, int32_t eventHubId) { device->getId(), eventHubId, device->getName().c_str(), device->getDescriptor().c_str()); } else { - ALOGI("Device removed: id=%d, eventHubId=%d, name='%s', descriptor='%s', sources=0x%08x", + ALOGI("Device removed: id=%d, eventHubId=%d, name='%s', descriptor='%s', sources=%s", device->getId(), eventHubId, device->getName().c_str(), - device->getDescriptor().c_str(), device->getSources()); + device->getDescriptor().c_str(), + inputEventSourceToString(device->getSources()).c_str()); } device->removeEventHubDevice(eventHubId); diff --git a/services/inputflinger/tests/Android.bp b/services/inputflinger/tests/Android.bp index 6da214506a..76500c577d 100644 --- a/services/inputflinger/tests/Android.bp +++ b/services/inputflinger/tests/Android.bp @@ -60,6 +60,7 @@ cc_test { }, static_libs: [ "libc++fs", + "libgmock", ], require_root: true, test_suites: ["device-tests"], diff --git a/services/inputflinger/tests/TestInputListener.cpp b/services/inputflinger/tests/TestInputListener.cpp index 6a26c636d9..57b382c718 100644 --- a/services/inputflinger/tests/TestInputListener.cpp +++ b/services/inputflinger/tests/TestInputListener.cpp @@ -69,6 +69,13 @@ void TestInputListener::assertNotifyMotionWasCalled(NotifyMotionArgs* outEventAr "Expected notifyMotion() to have been called.")); } +void TestInputListener::assertNotifyMotionWasCalled( + const ::testing::Matcher<NotifyMotionArgs>& matcher) { + NotifyMotionArgs outEventArgs; + ASSERT_NO_FATAL_FAILURE(assertNotifyMotionWasCalled(&outEventArgs)); + ASSERT_THAT(outEventArgs, matcher); +} + void TestInputListener::assertNotifyMotionWasNotCalled() { ASSERT_NO_FATAL_FAILURE( assertNotCalled<NotifyMotionArgs>("notifyMotion() should not be called.")); diff --git a/services/inputflinger/tests/TestInputListener.h b/services/inputflinger/tests/TestInputListener.h index 626cdfc8c8..0bdfc6b9fb 100644 --- a/services/inputflinger/tests/TestInputListener.h +++ b/services/inputflinger/tests/TestInputListener.h @@ -18,6 +18,7 @@ #define _UI_TEST_INPUT_LISTENER_H #include <android-base/thread_annotations.h> +#include <gmock/gmock.h> #include <gtest/gtest.h> #include "InputListener.h" @@ -48,6 +49,8 @@ public: void assertNotifyMotionWasCalled(NotifyMotionArgs* outEventArgs = nullptr); + void assertNotifyMotionWasCalled(const ::testing::Matcher<NotifyMotionArgs>& matcher); + void assertNotifyMotionWasNotCalled(); void assertNotifySwitchWasCalled(NotifySwitchArgs* outEventArgs = nullptr); diff --git a/services/inputflinger/tests/UnwantedInteractionBlocker_test.cpp b/services/inputflinger/tests/UnwantedInteractionBlocker_test.cpp index f2a39640b1..526e4eb7c1 100644 --- a/services/inputflinger/tests/UnwantedInteractionBlocker_test.cpp +++ b/services/inputflinger/tests/UnwantedInteractionBlocker_test.cpp @@ -16,13 +16,17 @@ #include "../UnwantedInteractionBlocker.h" #include <android-base/silent_death_test.h> +#include <gmock/gmock.h> #include <gtest/gtest.h> #include <gui/constants.h> #include <linux/input.h> #include <thread> +#include "ui/events/ozone/evdev/touch_filter/neural_stylus_palm_detection_filter.h" #include "TestInputListener.h" +using ::testing::AllOf; + namespace android { constexpr int32_t DEVICE_ID = 3; @@ -30,6 +34,8 @@ constexpr int32_t X_RESOLUTION = 11; constexpr int32_t Y_RESOLUTION = 11; constexpr int32_t MAJOR_RESOLUTION = 1; +const nsecs_t RESAMPLE_PERIOD = ::ui::kResamplePeriod.InNanoseconds(); + constexpr int POINTER_0_DOWN = AMOTION_EVENT_ACTION_POINTER_DOWN | (0 << AMOTION_EVENT_ACTION_POINTER_INDEX_SHIFT); constexpr int POINTER_1_DOWN = @@ -47,6 +53,23 @@ constexpr int MOVE = AMOTION_EVENT_ACTION_MOVE; constexpr int UP = AMOTION_EVENT_ACTION_UP; constexpr int CANCEL = AMOTION_EVENT_ACTION_CANCEL; +constexpr int32_t FLAG_CANCELED = AMOTION_EVENT_FLAG_CANCELED; + +MATCHER_P(WithAction, action, "MotionEvent with specified action") { + bool result = true; + if (action == CANCEL) { + result &= (arg.flags & FLAG_CANCELED) != 0; + } + result &= arg.action == action; + *result_listener << "expected to receive " << MotionEvent::actionToString(action) + << " but received " << MotionEvent::actionToString(arg.action) << " instead."; + return result; +} + +MATCHER_P(WithFlags, flags, "MotionEvent with specified flags") { + return arg.flags == flags; +} + static nsecs_t toNs(std::chrono::nanoseconds duration) { return duration.count(); } @@ -256,7 +279,7 @@ TEST(CancelSuppressedPointersTest, NewlySuppressedPointerIsCanceled) { /*newSuppressedPointerIds*/ {1}); ASSERT_EQ(2u, result.size()); assertArgs(result[0], POINTER_1_UP, {{0, {1, 2, 3}}, {1, {4, 5, 6}}, {2, {7, 8, 9}}}); - ASSERT_EQ(AMOTION_EVENT_FLAG_CANCELED, result[0].flags); + ASSERT_EQ(FLAG_CANCELED, result[0].flags); assertArgs(result[1], MOVE, {{0, {1, 2, 3}}, {2, {7, 8, 9}}}); } @@ -271,7 +294,7 @@ TEST(CancelSuppressedPointersTest, SingleSuppressedPointerIsCanceled) { /*newSuppressedPointerIds*/ {0}); ASSERT_EQ(1u, result.size()); assertArgs(result[0], CANCEL, {{0, {1, 2, 3}}}); - ASSERT_EQ(AMOTION_EVENT_FLAG_CANCELED, result[0].flags); + ASSERT_EQ(FLAG_CANCELED, result[0].flags); } /** @@ -286,7 +309,7 @@ TEST(CancelSuppressedPointersTest, SuppressedPointer1GoingUpIsCanceled) { /*newSuppressedPointerIds*/ {1}); ASSERT_EQ(1u, result.size()); assertArgs(result[0], POINTER_1_UP, {{0, {1, 2, 3}}, {1, {4, 5, 6}}, {2, {7, 8, 9}}}); - ASSERT_EQ(AMOTION_EVENT_FLAG_CANCELED, result[0].flags); + ASSERT_EQ(FLAG_CANCELED, result[0].flags); } /** @@ -301,7 +324,7 @@ TEST(CancelSuppressedPointersTest, SuppressedPointer0GoingUpIsCanceled) { /*newSuppressedPointerIds*/ {0}); ASSERT_EQ(1u, result.size()); assertArgs(result[0], POINTER_0_UP, {{0, {1, 2, 3}}, {1, {4, 5, 6}}}); - ASSERT_EQ(AMOTION_EVENT_FLAG_CANCELED, result[0].flags); + ASSERT_EQ(FLAG_CANCELED, result[0].flags); } /** @@ -316,7 +339,7 @@ TEST(CancelSuppressedPointersTest, TwoNewlySuppressedPointersAreBothCanceled) { /*newSuppressedPointerIds*/ {0, 1}); ASSERT_EQ(1u, result.size()); assertArgs(result[0], CANCEL, {{0, {1, 2, 3}}, {1, {4, 5, 6}}}); - ASSERT_EQ(AMOTION_EVENT_FLAG_CANCELED, result[0].flags); + ASSERT_EQ(FLAG_CANCELED, result[0].flags); } /** @@ -332,7 +355,7 @@ TEST(CancelSuppressedPointersTest, TwoPointersAreCanceledDuringPointerUp) { /*newSuppressedPointerIds*/ {0, 1}); ASSERT_EQ(1u, result.size()); assertArgs(result[0], CANCEL, {{0, {1, 2, 3}}}); - ASSERT_EQ(AMOTION_EVENT_FLAG_CANCELED, result[0].flags); + ASSERT_EQ(FLAG_CANCELED, result[0].flags); } /** @@ -573,6 +596,114 @@ TEST_F(UnwantedInteractionBlockerTest, DumpCanBeAccessedOnAnotherThread) { dumpThread.join(); } +/** + * Heuristic filter that's present in the palm rejection model blocks touches early if the size + * of the touch is large. This is an integration test that checks that this filter kicks in. + */ +TEST_F(UnwantedInteractionBlockerTest, HeuristicFilterWorks) { + mBlocker->notifyInputDevicesChanged({generateTestDeviceInfo()}); + // Small touch down + NotifyMotionArgs args1 = generateMotionArgs(0 /*downTime*/, 0 /*eventTime*/, DOWN, {{1, 2, 3}}); + mBlocker->notifyMotion(&args1); + mTestListener.assertNotifyMotionWasCalled(WithAction(DOWN)); + + // Large touch oval on the next move + NotifyMotionArgs args2 = + generateMotionArgs(0 /*downTime*/, RESAMPLE_PERIOD, MOVE, {{4, 5, 200}}); + mBlocker->notifyMotion(&args2); + mTestListener.assertNotifyMotionWasCalled(WithAction(MOVE)); + + // Lift up the touch to force the model to decide on whether it's a palm + NotifyMotionArgs args3 = + generateMotionArgs(0 /*downTime*/, 2 * RESAMPLE_PERIOD, UP, {{4, 5, 200}}); + mBlocker->notifyMotion(&args3); + mTestListener.assertNotifyMotionWasCalled(WithAction(CANCEL)); +} + +/** + * Send a stylus event that would have triggered the heuristic palm detector if it were a touch + * event. However, since it's a stylus event, it should propagate without being canceled through + * the blocker. + * This is similar to `HeuristicFilterWorks` test, but for stylus tool. + */ +TEST_F(UnwantedInteractionBlockerTest, StylusIsNotBlocked) { + InputDeviceInfo info = generateTestDeviceInfo(); + info.addSource(AINPUT_SOURCE_STYLUS); + mBlocker->notifyInputDevicesChanged({info}); + NotifyMotionArgs args1 = generateMotionArgs(0 /*downTime*/, 0 /*eventTime*/, DOWN, {{1, 2, 3}}); + args1.pointerProperties[0].toolType = AMOTION_EVENT_TOOL_TYPE_STYLUS; + mBlocker->notifyMotion(&args1); + mTestListener.assertNotifyMotionWasCalled(WithAction(DOWN)); + + // Move the stylus, setting large TOUCH_MAJOR/TOUCH_MINOR dimensions + NotifyMotionArgs args2 = + generateMotionArgs(0 /*downTime*/, RESAMPLE_PERIOD, MOVE, {{4, 5, 200}}); + args2.pointerProperties[0].toolType = AMOTION_EVENT_TOOL_TYPE_STYLUS; + mBlocker->notifyMotion(&args2); + mTestListener.assertNotifyMotionWasCalled(WithAction(MOVE)); + + // Lift up the stylus. If it were a touch event, this would force the model to decide on whether + // it's a palm. + NotifyMotionArgs args3 = + generateMotionArgs(0 /*downTime*/, 2 * RESAMPLE_PERIOD, UP, {{4, 5, 200}}); + args3.pointerProperties[0].toolType = AMOTION_EVENT_TOOL_TYPE_STYLUS; + mBlocker->notifyMotion(&args3); + mTestListener.assertNotifyMotionWasCalled(WithAction(UP)); +} + +/** + * Send a mixed touch and stylus event. + * The touch event goes first, and is a palm. The stylus event goes down after. + * Stylus event should continue to work even after touch is detected as a palm. + */ +TEST_F(UnwantedInteractionBlockerTest, TouchIsBlockedWhenMixedWithStylus) { + InputDeviceInfo info = generateTestDeviceInfo(); + info.addSource(AINPUT_SOURCE_STYLUS); + mBlocker->notifyInputDevicesChanged({info}); + + // Touch down + NotifyMotionArgs args1 = generateMotionArgs(0 /*downTime*/, 0 /*eventTime*/, DOWN, {{1, 2, 3}}); + mBlocker->notifyMotion(&args1); + mTestListener.assertNotifyMotionWasCalled(WithAction(DOWN)); + + // Stylus pointer down + NotifyMotionArgs args2 = generateMotionArgs(0 /*downTime*/, RESAMPLE_PERIOD, POINTER_1_DOWN, + {{1, 2, 3}, {10, 20, 30}}); + args2.pointerProperties[1].toolType = AMOTION_EVENT_TOOL_TYPE_STYLUS; + mBlocker->notifyMotion(&args2); + mTestListener.assertNotifyMotionWasCalled(WithAction(POINTER_1_DOWN)); + + // Large touch oval on the next finger move + NotifyMotionArgs args3 = generateMotionArgs(0 /*downTime*/, 2 * RESAMPLE_PERIOD, MOVE, + {{1, 2, 300}, {11, 21, 30}}); + args3.pointerProperties[1].toolType = AMOTION_EVENT_TOOL_TYPE_STYLUS; + mBlocker->notifyMotion(&args3); + mTestListener.assertNotifyMotionWasCalled(WithAction(MOVE)); + + // Lift up the finger pointer. It should be canceled due to the heuristic filter. + NotifyMotionArgs args4 = generateMotionArgs(0 /*downTime*/, 3 * RESAMPLE_PERIOD, POINTER_0_UP, + {{1, 2, 300}, {11, 21, 30}}); + args4.pointerProperties[1].toolType = AMOTION_EVENT_TOOL_TYPE_STYLUS; + mBlocker->notifyMotion(&args4); + mTestListener.assertNotifyMotionWasCalled( + AllOf(WithAction(POINTER_0_UP), WithFlags(FLAG_CANCELED))); + + NotifyMotionArgs args5 = + generateMotionArgs(0 /*downTime*/, 4 * RESAMPLE_PERIOD, MOVE, {{12, 22, 30}}); + args5.pointerProperties[0].id = args4.pointerProperties[1].id; + args5.pointerProperties[0].toolType = AMOTION_EVENT_TOOL_TYPE_STYLUS; + mBlocker->notifyMotion(&args5); + mTestListener.assertNotifyMotionWasCalled(WithAction(MOVE)); + + // Lift up the stylus pointer + NotifyMotionArgs args6 = + generateMotionArgs(0 /*downTime*/, 5 * RESAMPLE_PERIOD, UP, {{4, 5, 200}}); + args6.pointerProperties[0].id = args4.pointerProperties[1].id; + args6.pointerProperties[0].toolType = AMOTION_EVENT_TOOL_TYPE_STYLUS; + mBlocker->notifyMotion(&args6); + mTestListener.assertNotifyMotionWasCalled(WithAction(UP)); +} + using UnwantedInteractionBlockerTestDeathTest = UnwantedInteractionBlockerTest; /** @@ -672,7 +803,7 @@ TEST_F(PalmRejectorTest, TwoPointersAreCanceled) { {{1433.0, 751.0, 44.0}, {1070.0, 771.0, 13.0}})); ASSERT_EQ(2u, argsList.size()); ASSERT_EQ(POINTER_0_UP, argsList[0].action); - ASSERT_EQ(AMOTION_EVENT_FLAG_CANCELED, argsList[0].flags); + ASSERT_EQ(FLAG_CANCELED, argsList[0].flags); ASSERT_EQ(MOVE, argsList[1].action); ASSERT_EQ(1u, argsList[1].pointerCount); ASSERT_EQ(0, argsList[1].flags); @@ -849,7 +980,7 @@ TEST_F(PalmRejectorFakeFilterTest, OneOfTwoPointersIsCanceled) { ASSERT_EQ(2u, argsList.size()); // First event - cancel pointer 1 ASSERT_EQ(POINTER_1_UP, argsList[0].action); - ASSERT_EQ(AMOTION_EVENT_FLAG_CANCELED, argsList[0].flags); + ASSERT_EQ(FLAG_CANCELED, argsList[0].flags); // Second event - send MOVE for the remaining pointer ASSERT_EQ(MOVE, argsList[1].action); ASSERT_EQ(0, argsList[1].flags); @@ -890,7 +1021,7 @@ TEST_F(PalmRejectorFakeFilterTest, NewDownEventAfterCancel) { // Cancel all ASSERT_EQ(CANCEL, argsList[0].action); ASSERT_EQ(2u, argsList[0].pointerCount); - ASSERT_EQ(AMOTION_EVENT_FLAG_CANCELED, argsList[0].flags); + ASSERT_EQ(FLAG_CANCELED, argsList[0].flags); // Future move events are ignored argsList = mPalmRejector->processMotion( @@ -936,7 +1067,7 @@ TEST_F(PalmRejectorFakeFilterTest, TwoPointersCanceledWhenOnePointerGoesUp) { {{1414.0, 702.0, 41.0}, {1059.0, 731.0, 12.0}})); ASSERT_EQ(1u, argsList.size()); ASSERT_EQ(CANCEL, argsList[0].action) << MotionEvent::actionToString(argsList[0].action); - ASSERT_EQ(AMOTION_EVENT_FLAG_CANCELED, argsList[0].flags); + ASSERT_EQ(FLAG_CANCELED, argsList[0].flags); // Future move events should not go to the listener. argsList = mPalmRejector->processMotion( @@ -970,7 +1101,7 @@ TEST_F(PalmRejectorFakeFilterTest, CancelTwoPointers) { {{1417.0, 685.0, 41.0}, {1060, 700, 10.0}})); ASSERT_EQ(2u, argsList.size()); ASSERT_EQ(POINTER_1_UP, argsList[0].action); - ASSERT_EQ(AMOTION_EVENT_FLAG_CANCELED, argsList[0].flags); + ASSERT_EQ(FLAG_CANCELED, argsList[0].flags); ASSERT_EQ(MOVE, argsList[1].action) << MotionEvent::actionToString(argsList[1].action); ASSERT_EQ(0, argsList[1].flags); diff --git a/services/surfaceflinger/BufferStateLayer.cpp b/services/surfaceflinger/BufferStateLayer.cpp index 51a54455d1..cce6ad7fe0 100644 --- a/services/surfaceflinger/BufferStateLayer.cpp +++ b/services/surfaceflinger/BufferStateLayer.cpp @@ -1109,7 +1109,29 @@ bool BufferStateLayer::isVisible() const { } std::optional<compositionengine::LayerFE::LayerSettings> BufferStateLayer::prepareClientComposition( - compositionengine::LayerFE::ClientCompositionTargetSettings& targetSettings) { + compositionengine::LayerFE::ClientCompositionTargetSettings& targetSettings) const { + std::optional<compositionengine::LayerFE::LayerSettings> layerSettings = + prepareClientCompositionInternal(targetSettings); + // Nothing to render. + if (!layerSettings) { + return {}; + } + + // HWC requests to clear this layer. + if (targetSettings.clearContent) { + prepareClearClientComposition(*layerSettings, false /* blackout */); + return *layerSettings; + } + + // set the shadow for the layer if needed + prepareShadowClientComposition(*layerSettings, targetSettings.viewport); + + return *layerSettings; +} + +std::optional<compositionengine::LayerFE::LayerSettings> +BufferStateLayer::prepareClientCompositionInternal( + compositionengine::LayerFE::ClientCompositionTargetSettings& targetSettings) const { ATRACE_CALL(); std::optional<compositionengine::LayerFE::LayerSettings> result = @@ -1559,7 +1581,7 @@ sp<GraphicBuffer> BufferStateLayer::getBuffer() const { return mBufferInfo.mBuffer ? mBufferInfo.mBuffer->getBuffer() : nullptr; } -void BufferStateLayer::getDrawingTransformMatrix(bool filteringEnabled, float outMatrix[16]) { +void BufferStateLayer::getDrawingTransformMatrix(bool filteringEnabled, float outMatrix[16]) const { sp<GraphicBuffer> buffer = getBuffer(); if (!buffer) { ALOGE("Buffer should not be null!"); diff --git a/services/surfaceflinger/BufferStateLayer.h b/services/surfaceflinger/BufferStateLayer.h index 8bad3d230c..a0f13e21d4 100644 --- a/services/surfaceflinger/BufferStateLayer.h +++ b/services/surfaceflinger/BufferStateLayer.h @@ -159,6 +159,9 @@ public: std::atomic<int32_t>* getPendingBufferCounter() override { return &mPendingBufferTransactions; } std::string getPendingBufferCounterName() override { return mBlastTransactionName; } + std::optional<compositionengine::LayerFE::LayerSettings> prepareClientComposition( + compositionengine::LayerFE::ClientCompositionTargetSettings&) const override; + protected: void gatherBufferInfo(); void onSurfaceFrameCreated(const std::shared_ptr<frametimeline::SurfaceFrame>& surfaceFrame); @@ -188,9 +191,6 @@ protected: BufferInfo mBufferInfo; - std::optional<compositionengine::LayerFE::LayerSettings> prepareClientComposition( - compositionengine::LayerFE::ClientCompositionTargetSettings&) override; - /* * compositionengine::LayerFE overrides */ @@ -236,7 +236,7 @@ private: // Computes the transform matrix using the setFilteringEnabled to determine whether the // transform matrix should be computed for use with bilinear filtering. - void getDrawingTransformMatrix(bool filteringEnabled, float outMatrix[16]); + void getDrawingTransformMatrix(bool filteringEnabled, float outMatrix[16]) const; std::unique_ptr<compositionengine::LayerFECompositionState> mCompositionState; @@ -270,6 +270,9 @@ private: const sp<Fence>& releaseFence, uint32_t currentMaxAcquiredBufferCount); + std::optional<compositionengine::LayerFE::LayerSettings> prepareClientCompositionInternal( + compositionengine::LayerFE::ClientCompositionTargetSettings&) const; + ReleaseCallbackId mPreviousReleaseCallbackId = ReleaseCallbackId::INVALID_ID; uint64_t mPreviousReleasedFrameNumber = 0; diff --git a/services/surfaceflinger/CompositionEngine/include/compositionengine/LayerFE.h b/services/surfaceflinger/CompositionEngine/include/compositionengine/LayerFE.h index ec610c1b1d..f93fd99f29 100644 --- a/services/surfaceflinger/CompositionEngine/include/compositionengine/LayerFE.h +++ b/services/surfaceflinger/CompositionEngine/include/compositionengine/LayerFE.h @@ -150,11 +150,11 @@ public: uint64_t frameNumber = 0; }; - // Returns the z-ordered list of LayerSettings to pass to RenderEngine::drawLayers. The list - // may contain shadows casted by the layer or the content of the layer itself. If the layer - // does not render then an empty list will be returned. - virtual std::vector<LayerSettings> prepareClientCompositionList( - ClientCompositionTargetSettings&) = 0; + // Returns the LayerSettings to pass to RenderEngine::drawLayers. The state may contain shadows + // casted by the layer or the content of the layer itself. If the layer does not render then an + // empty optional will be returned. + virtual std::optional<LayerSettings> prepareClientComposition( + ClientCompositionTargetSettings&) const = 0; // Called after the layer is displayed to update the presentation fence virtual void onLayerDisplayed(ftl::SharedFuture<FenceResult>) = 0; diff --git a/services/surfaceflinger/CompositionEngine/include/compositionengine/OutputLayer.h b/services/surfaceflinger/CompositionEngine/include/compositionengine/OutputLayer.h index bf5184e997..6d0c395b2f 100644 --- a/services/surfaceflinger/CompositionEngine/include/compositionengine/OutputLayer.h +++ b/services/surfaceflinger/CompositionEngine/include/compositionengine/OutputLayer.h @@ -126,9 +126,9 @@ public: // Returns true if the composition settings scale pixels virtual bool needsFiltering() const = 0; - // Returns a composition list to be used by RenderEngine if the layer has been overridden + // Returns LayerSettings to be used by RenderEngine if the layer has been overridden // during the composition process - virtual std::vector<LayerFE::LayerSettings> getOverrideCompositionList() const = 0; + virtual std::optional<LayerFE::LayerSettings> getOverrideCompositionSettings() const = 0; // Debugging virtual void dump(std::string& result) const = 0; diff --git a/services/surfaceflinger/CompositionEngine/include/compositionengine/impl/OutputLayer.h b/services/surfaceflinger/CompositionEngine/include/compositionengine/impl/OutputLayer.h index ecd432f629..6d4abf9f47 100644 --- a/services/surfaceflinger/CompositionEngine/include/compositionengine/impl/OutputLayer.h +++ b/services/surfaceflinger/CompositionEngine/include/compositionengine/impl/OutputLayer.h @@ -18,6 +18,7 @@ #include <cstdint> #include <memory> +#include <optional> #include <string> #include <compositionengine/LayerFE.h> @@ -57,7 +58,7 @@ public: void prepareForDeviceLayerRequests() override; void applyDeviceLayerRequest(Hwc2::IComposerClient::LayerRequest request) override; bool needsFiltering() const override; - std::vector<LayerFE::LayerSettings> getOverrideCompositionList() const override; + std::optional<LayerFE::LayerSettings> getOverrideCompositionSettings() const override; void dump(std::string&) const override; virtual FloatRect calculateOutputSourceCrop(uint32_t internalDisplayRotationFlags) const; diff --git a/services/surfaceflinger/CompositionEngine/include/compositionengine/mock/LayerFE.h b/services/surfaceflinger/CompositionEngine/include/compositionengine/mock/LayerFE.h index 1c5c10f823..2b704e697f 100644 --- a/services/surfaceflinger/CompositionEngine/include/compositionengine/mock/LayerFE.h +++ b/services/surfaceflinger/CompositionEngine/include/compositionengine/mock/LayerFE.h @@ -45,9 +45,9 @@ public: MOCK_METHOD1(onPreComposition, bool(nsecs_t)); MOCK_METHOD1(prepareCompositionState, void(compositionengine::LayerFE::StateSubset)); - MOCK_METHOD1(prepareClientCompositionList, - std::vector<compositionengine::LayerFE::LayerSettings>( - compositionengine::LayerFE::ClientCompositionTargetSettings&)); + MOCK_CONST_METHOD1(prepareClientComposition, + std::optional<compositionengine::LayerFE::LayerSettings>( + compositionengine::LayerFE::ClientCompositionTargetSettings&)); MOCK_METHOD(void, onLayerDisplayed, (ftl::SharedFuture<FenceResult>), (override)); diff --git a/services/surfaceflinger/CompositionEngine/include/compositionengine/mock/OutputLayer.h b/services/surfaceflinger/CompositionEngine/include/compositionengine/mock/OutputLayer.h index a6cb811468..c22f1bf260 100644 --- a/services/surfaceflinger/CompositionEngine/include/compositionengine/mock/OutputLayer.h +++ b/services/surfaceflinger/CompositionEngine/include/compositionengine/mock/OutputLayer.h @@ -16,6 +16,8 @@ #pragma once +#include <optional> + #include <compositionengine/CompositionEngine.h> #include <compositionengine/LayerFE.h> #include <compositionengine/Output.h> @@ -51,7 +53,7 @@ public: MOCK_METHOD0(prepareForDeviceLayerRequests, void()); MOCK_METHOD1(applyDeviceLayerRequest, void(Hwc2::IComposerClient::LayerRequest request)); MOCK_CONST_METHOD0(needsFiltering, bool()); - MOCK_CONST_METHOD0(getOverrideCompositionList, std::vector<LayerFE::LayerSettings>()); + MOCK_CONST_METHOD0(getOverrideCompositionSettings, std::optional<LayerFE::LayerSettings>()); MOCK_CONST_METHOD1(dump, void(std::string&)); }; diff --git a/services/surfaceflinger/CompositionEngine/src/Output.cpp b/services/surfaceflinger/CompositionEngine/src/Output.cpp index 8ebc5b1702..6edc00f1e1 100644 --- a/services/surfaceflinger/CompositionEngine/src/Output.cpp +++ b/services/surfaceflinger/CompositionEngine/src/Output.cpp @@ -1366,7 +1366,7 @@ std::vector<LayerFE::LayerSettings> Output::generateClientCompositionRequests( bool firstLayer = true; bool disableBlurs = false; - sp<GraphicBuffer> previousOverrideBuffer = nullptr; + uint64_t previousOverrideBufferId = 0; for (auto* layer : getOutputLayersOrderedByZ()) { const auto& layerState = layer->getState(); @@ -1402,11 +1402,10 @@ std::vector<LayerFE::LayerSettings> Output::generateClientCompositionRequests( !layerState.visibleRegion.subtract(layerState.shadowRegion).isEmpty(); if (clientComposition || clearClientComposition) { - std::vector<LayerFE::LayerSettings> results; - if (layer->getState().overrideInfo.buffer != nullptr) { - if (layer->getState().overrideInfo.buffer->getBuffer() != previousOverrideBuffer) { - results = layer->getOverrideCompositionList(); - previousOverrideBuffer = layer->getState().overrideInfo.buffer->getBuffer(); + if (auto overrideSettings = layer->getOverrideCompositionSettings()) { + if (overrideSettings->bufferId != previousOverrideBufferId) { + previousOverrideBufferId = overrideSettings->bufferId; + clientCompositionLayers.push_back(std::move(*overrideSettings)); ALOGV("Replacing [%s] with override in RE", layer->getLayerFE().getDebugName()); } else { ALOGV("Skipping redundant override buffer for [%s] in RE", @@ -1432,20 +1431,18 @@ std::vector<LayerFE::LayerSettings> Output::generateClientCompositionRequests( .clearContent = !clientComposition, .blurSetting = blurSetting, .whitePointNits = layerState.whitePointNits}; - results = layerFE.prepareClientCompositionList(targetSettings); - if (realContentIsVisible && !results.empty()) { - layer->editState().clientCompositionTimestamp = systemTime(); + if (auto clientCompositionSettings = + layerFE.prepareClientComposition(targetSettings)) { + clientCompositionLayers.push_back(std::move(*clientCompositionSettings)); + if (realContentIsVisible) { + layer->editState().clientCompositionTimestamp = systemTime(); + } } } if (clientComposition) { outLayerFEs.push_back(&layerFE); } - - clientCompositionLayers.insert(clientCompositionLayers.end(), - std::make_move_iterator(results.begin()), - std::make_move_iterator(results.end())); - results.clear(); } firstLayer = false; diff --git a/services/surfaceflinger/CompositionEngine/src/OutputLayer.cpp b/services/surfaceflinger/CompositionEngine/src/OutputLayer.cpp index 1bb9d0eb63..a39c527655 100644 --- a/services/surfaceflinger/CompositionEngine/src/OutputLayer.cpp +++ b/services/surfaceflinger/CompositionEngine/src/OutputLayer.cpp @@ -787,7 +787,7 @@ bool OutputLayer::needsFiltering() const { sourceCrop.getWidth() != displayFrame.getWidth(); } -std::vector<LayerFE::LayerSettings> OutputLayer::getOverrideCompositionList() const { +std::optional<LayerFE::LayerSettings> OutputLayer::getOverrideCompositionSettings() const { if (getState().overrideInfo.buffer == nullptr) { return {}; } @@ -816,7 +816,7 @@ std::vector<LayerFE::LayerSettings> OutputLayer::getOverrideCompositionList() co settings.alpha = 1.0f; settings.whitePointNits = getOutput().getState().sdrWhitePointNits; - return {static_cast<LayerFE::LayerSettings>(settings)}; + return settings; } void OutputLayer::dump(std::string& out) const { diff --git a/services/surfaceflinger/CompositionEngine/src/planner/CachedSet.cpp b/services/surfaceflinger/CompositionEngine/src/planner/CachedSet.cpp index 641b806aec..9058f6709f 100644 --- a/services/surfaceflinger/CompositionEngine/src/planner/CachedSet.cpp +++ b/services/surfaceflinger/CompositionEngine/src/planner/CachedSet.cpp @@ -190,11 +190,11 @@ void CachedSet::render(renderengine::RenderEngine& renderEngine, TexturePool& te std::vector<renderengine::LayerSettings> layerSettings; renderengine::LayerSettings highlight; for (const auto& layer : mLayers) { - const auto clientCompositionList = - layer.getState()->getOutputLayer()->getLayerFE().prepareClientCompositionList( - targetSettings); - layerSettings.insert(layerSettings.end(), clientCompositionList.cbegin(), - clientCompositionList.cend()); + if (auto clientCompositionSettings = + layer.getState()->getOutputLayer()->getLayerFE().prepareClientComposition( + targetSettings)) { + layerSettings.push_back(std::move(*clientCompositionSettings)); + } } renderengine::LayerSettings blurLayerSettings; @@ -202,43 +202,40 @@ void CachedSet::render(renderengine::RenderEngine& renderEngine, TexturePool& te auto blurSettings = targetSettings; blurSettings.blurSetting = LayerFE::ClientCompositionTargetSettings::BlurSetting::BackgroundBlurOnly; - auto clientCompositionList = - mBlurLayer->getOutputLayer()->getLayerFE().prepareClientCompositionList( - blurSettings); - blurLayerSettings = clientCompositionList.back(); + + auto blurLayerSettings = + mBlurLayer->getOutputLayer()->getLayerFE().prepareClientComposition(blurSettings); // This mimics Layer::prepareClearClientComposition - blurLayerSettings.skipContentDraw = true; - blurLayerSettings.name = std::string("blur layer"); + blurLayerSettings->skipContentDraw = true; + blurLayerSettings->name = std::string("blur layer"); // Clear out the shadow settings - blurLayerSettings.shadow = {}; - layerSettings.push_back(blurLayerSettings); + blurLayerSettings->shadow = {}; + layerSettings.push_back(std::move(*blurLayerSettings)); } - renderengine::LayerSettings holePunchSettings; - renderengine::LayerSettings holePunchBackgroundSettings; if (mHolePunchLayer) { auto& layerFE = mHolePunchLayer->getOutputLayer()->getLayerFE(); - auto clientCompositionList = layerFE.prepareClientCompositionList(targetSettings); - // Assume that the final layer contains the buffer that we want to - // replace with a hole punch. - holePunchSettings = clientCompositionList.back(); + + auto holePunchSettings = layerFE.prepareClientComposition(targetSettings); // This mimics Layer::prepareClearClientComposition - holePunchSettings.source.buffer.buffer = nullptr; - holePunchSettings.source.solidColor = half3(0.0f, 0.0f, 0.0f); - holePunchSettings.disableBlending = true; - holePunchSettings.alpha = 0.0f; - holePunchSettings.name = + holePunchSettings->source.buffer.buffer = nullptr; + holePunchSettings->source.solidColor = half3(0.0f, 0.0f, 0.0f); + holePunchSettings->disableBlending = true; + holePunchSettings->alpha = 0.0f; + holePunchSettings->name = android::base::StringPrintf("hole punch layer for %s", layerFE.getDebugName()); - layerSettings.push_back(holePunchSettings); // Add a solid background as the first layer in case there is no opaque // buffer behind the punch hole + renderengine::LayerSettings holePunchBackgroundSettings; holePunchBackgroundSettings.alpha = 1.0f; holePunchBackgroundSettings.name = std::string("holePunchBackground"); - holePunchBackgroundSettings.geometry.boundaries = holePunchSettings.geometry.boundaries; + holePunchBackgroundSettings.geometry.boundaries = holePunchSettings->geometry.boundaries; holePunchBackgroundSettings.geometry.positionTransform = - holePunchSettings.geometry.positionTransform; - layerSettings.emplace(layerSettings.begin(), holePunchBackgroundSettings); + holePunchSettings->geometry.positionTransform; + layerSettings.emplace(layerSettings.begin(), std::move(holePunchBackgroundSettings)); + + layerSettings.push_back(std::move(*holePunchSettings)); } if (sDebugHighlighLayers) { diff --git a/services/surfaceflinger/CompositionEngine/tests/OutputTest.cpp b/services/surfaceflinger/CompositionEngine/tests/OutputTest.cpp index ad0fb9d6c4..163a11ccd1 100644 --- a/services/surfaceflinger/CompositionEngine/tests/OutputTest.cpp +++ b/services/surfaceflinger/CompositionEngine/tests/OutputTest.cpp @@ -4308,6 +4308,8 @@ struct GenerateClientCompositionRequestsTest : public testing::Test { struct Layer { Layer() { + EXPECT_CALL(mOutputLayer, getOverrideCompositionSettings()) + .WillRepeatedly(Return(std::nullopt)); EXPECT_CALL(mOutputLayer, getState()).WillRepeatedly(ReturnRef(mOutputLayerState)); EXPECT_CALL(mOutputLayer, editState()).WillRepeatedly(ReturnRef(mOutputLayerState)); EXPECT_CALL(mOutputLayer, getLayerFE()).WillRepeatedly(ReturnRef(*mLayerFE)); @@ -4405,23 +4407,18 @@ TEST_F(GenerateClientCompositionRequestsTest_ThreeLayers, requiresVisibleRegionA } TEST_F(GenerateClientCompositionRequestsTest_ThreeLayers, gathersClientCompositionRequests) { - LayerFE::LayerSettings mShadowSettings; - mShadowSettings.source.solidColor = {0.1f, 0.1f, 0.1f}; - - EXPECT_CALL(*mLayers[0].mLayerFE, prepareClientCompositionList(_)) - .WillOnce(Return(std::vector<LayerFE::LayerSettings>())); - EXPECT_CALL(*mLayers[1].mLayerFE, prepareClientCompositionList(_)) - .WillOnce(Return(std::vector<LayerFE::LayerSettings>({mLayers[1].mLayerSettings}))); - EXPECT_CALL(*mLayers[2].mLayerFE, prepareClientCompositionList(_)) - .WillOnce(Return(std::vector<LayerFE::LayerSettings>( - {mShadowSettings, mLayers[2].mLayerSettings}))); + EXPECT_CALL(*mLayers[0].mLayerFE, prepareClientComposition(_)) + .WillOnce(Return(std::optional<LayerFE::LayerSettings>())); + EXPECT_CALL(*mLayers[1].mLayerFE, prepareClientComposition(_)) + .WillOnce(Return(std::optional<LayerFE::LayerSettings>(mLayers[1].mLayerSettings))); + EXPECT_CALL(*mLayers[2].mLayerFE, prepareClientComposition(_)) + .WillOnce(Return(std::optional<LayerFE::LayerSettings>(mLayers[2].mLayerSettings))); auto requests = mOutput.generateClientCompositionRequestsHelper(false /* supportsProtectedContent */, kDisplayDataspace); - ASSERT_EQ(3u, requests.size()); + ASSERT_EQ(2u, requests.size()); EXPECT_EQ(mLayers[1].mLayerSettings, requests[0]); - EXPECT_EQ(mShadowSettings, requests[1]); - EXPECT_EQ(mLayers[2].mLayerSettings, requests[2]); + EXPECT_EQ(mLayers[2].mLayerSettings, requests[1]); // Check that a timestamp was set for the layers that generated requests EXPECT_TRUE(0 == mLayers[0].mOutputLayerState.clientCompositionTimestamp); @@ -4438,27 +4435,21 @@ MATCHER_P(ClientCompositionTargetSettingsBlurSettingsEq, expectedBlurSetting, "" } TEST_F(GenerateClientCompositionRequestsTest_ThreeLayers, overridesBlur) { - LayerFE::LayerSettings mShadowSettings; - mShadowSettings.source.solidColor = {0.1f, 0.1f, 0.1f}; - mLayers[2].mOutputLayerState.overrideInfo.disableBackgroundBlur = true; - EXPECT_CALL(*mLayers[0].mLayerFE, prepareClientCompositionList(_)) - .WillOnce(Return(std::vector<LayerFE::LayerSettings>())); - EXPECT_CALL(*mLayers[1].mLayerFE, prepareClientCompositionList(_)) - .WillOnce(Return(std::vector<LayerFE::LayerSettings>({mLayers[1].mLayerSettings}))); + EXPECT_CALL(*mLayers[0].mLayerFE, prepareClientComposition(_)) + .WillOnce(Return(std::optional<LayerFE::LayerSettings>())); + EXPECT_CALL(*mLayers[1].mLayerFE, prepareClientComposition(_)) + .WillOnce(Return(std::optional<LayerFE::LayerSettings>(mLayers[1].mLayerSettings))); EXPECT_CALL(*mLayers[2].mLayerFE, - prepareClientCompositionList(ClientCompositionTargetSettingsBlurSettingsEq( + prepareClientComposition(ClientCompositionTargetSettingsBlurSettingsEq( LayerFE::ClientCompositionTargetSettings::BlurSetting::BlurRegionsOnly))) - .WillOnce(Return(std::vector<LayerFE::LayerSettings>( - {mShadowSettings, mLayers[2].mLayerSettings}))); - + .WillOnce(Return(std::optional<LayerFE::LayerSettings>(mLayers[2].mLayerSettings))); auto requests = mOutput.generateClientCompositionRequestsHelper(false /* supportsProtectedContent */, kDisplayDataspace); - ASSERT_EQ(3u, requests.size()); + ASSERT_EQ(2u, requests.size()); EXPECT_EQ(mLayers[1].mLayerSettings, requests[0]); - EXPECT_EQ(mShadowSettings, requests[1]); - EXPECT_EQ(mLayers[2].mLayerSettings, requests[2]); + EXPECT_EQ(mLayers[2].mLayerSettings, requests[1]); // Check that a timestamp was set for the layers that generated requests EXPECT_TRUE(0 == mLayers[0].mOutputLayerState.clientCompositionTimestamp); @@ -4480,8 +4471,8 @@ TEST_F(GenerateClientCompositionRequestsTest_ThreeLayers, mLayers[1].mLayerFEState.isOpaque = true; mLayers[2].mLayerFEState.isOpaque = true; - EXPECT_CALL(*mLayers[2].mLayerFE, prepareClientCompositionList(_)) - .WillOnce(Return(std::vector<LayerFE::LayerSettings>({mLayers[2].mLayerSettings}))); + EXPECT_CALL(*mLayers[2].mLayerFE, prepareClientComposition(_)) + .WillOnce(Return(std::optional<LayerFE::LayerSettings>(mLayers[2].mLayerSettings))); auto requests = mOutput.generateClientCompositionRequestsHelper(false /* supportsProtectedContent */, kDisplayDataspace); @@ -4503,8 +4494,8 @@ TEST_F(GenerateClientCompositionRequestsTest_ThreeLayers, mLayers[1].mLayerFEState.isOpaque = false; mLayers[2].mLayerFEState.isOpaque = false; - EXPECT_CALL(*mLayers[2].mLayerFE, prepareClientCompositionList(_)) - .WillOnce(Return(std::vector<LayerFE::LayerSettings>({mLayers[2].mLayerSettings}))); + EXPECT_CALL(*mLayers[2].mLayerFE, prepareClientComposition(_)) + .WillOnce(Return(std::optional<LayerFE::LayerSettings>(mLayers[2].mLayerSettings))); auto requests = mOutput.generateClientCompositionRequestsHelper(false /* supportsProtectedContent */, kDisplayDataspace); @@ -4562,10 +4553,10 @@ TEST_F(GenerateClientCompositionRequestsTest_ThreeLayers, clearsHWCLayersIfOpaqu mBlackoutSettings.alpha = 0.f; mBlackoutSettings.disableBlending = true; - EXPECT_CALL(*mLayers[1].mLayerFE, prepareClientCompositionList(Eq(ByRef(layer1TargetSettings)))) - .WillOnce(Return(std::vector<LayerFE::LayerSettings>({mBlackoutSettings}))); - EXPECT_CALL(*mLayers[2].mLayerFE, prepareClientCompositionList(Eq(ByRef(layer2TargetSettings)))) - .WillOnce(Return(std::vector<LayerFE::LayerSettings>({mLayers[2].mLayerSettings}))); + EXPECT_CALL(*mLayers[1].mLayerFE, prepareClientComposition(Eq(ByRef(layer1TargetSettings)))) + .WillOnce(Return(std::optional<LayerFE::LayerSettings>(mBlackoutSettings))); + EXPECT_CALL(*mLayers[2].mLayerFE, prepareClientComposition(Eq(ByRef(layer2TargetSettings)))) + .WillOnce(Return(std::optional<LayerFE::LayerSettings>(mLayers[2].mLayerSettings))); auto requests = mOutput.generateClientCompositionRequestsHelper(false /* supportsProtectedContent */, kDisplayDataspace); @@ -4620,12 +4611,12 @@ TEST_F(GenerateClientCompositionRequestsTest_ThreeLayers, kLayerWhitePointNits, }; - EXPECT_CALL(*mLayers[0].mLayerFE, prepareClientCompositionList(Eq(ByRef(layer0TargetSettings)))) - .WillOnce(Return(std::vector<LayerFE::LayerSettings>())); - EXPECT_CALL(*mLayers[1].mLayerFE, prepareClientCompositionList(Eq(ByRef(layer1TargetSettings)))) - .WillOnce(Return(std::vector<LayerFE::LayerSettings>())); - EXPECT_CALL(*mLayers[2].mLayerFE, prepareClientCompositionList(Eq(ByRef(layer2TargetSettings)))) - .WillOnce(Return(std::vector<LayerFE::LayerSettings>())); + EXPECT_CALL(*mLayers[0].mLayerFE, prepareClientComposition(Eq(ByRef(layer0TargetSettings)))) + .WillOnce(Return(std::optional<LayerFE::LayerSettings>())); + EXPECT_CALL(*mLayers[1].mLayerFE, prepareClientComposition(Eq(ByRef(layer1TargetSettings)))) + .WillOnce(Return(std::optional<LayerFE::LayerSettings>())); + EXPECT_CALL(*mLayers[2].mLayerFE, prepareClientComposition(Eq(ByRef(layer2TargetSettings)))) + .WillOnce(Return(std::optional<LayerFE::LayerSettings>())); static_cast<void>( mOutput.generateClientCompositionRequestsHelper(false /* supportsProtectedContent */, @@ -4674,12 +4665,12 @@ TEST_F(GenerateClientCompositionRequestsTest_ThreeLayers, kLayerWhitePointNits, }; - EXPECT_CALL(*mLayers[0].mLayerFE, prepareClientCompositionList(Eq(ByRef(layer0TargetSettings)))) - .WillOnce(Return(std::vector<LayerFE::LayerSettings>())); - EXPECT_CALL(*mLayers[1].mLayerFE, prepareClientCompositionList(Eq(ByRef(layer1TargetSettings)))) - .WillOnce(Return(std::vector<LayerFE::LayerSettings>())); - EXPECT_CALL(*mLayers[2].mLayerFE, prepareClientCompositionList(Eq(ByRef(layer2TargetSettings)))) - .WillOnce(Return(std::vector<LayerFE::LayerSettings>())); + EXPECT_CALL(*mLayers[0].mLayerFE, prepareClientComposition(Eq(ByRef(layer0TargetSettings)))) + .WillOnce(Return(std::optional<LayerFE::LayerSettings>())); + EXPECT_CALL(*mLayers[1].mLayerFE, prepareClientComposition(Eq(ByRef(layer1TargetSettings)))) + .WillOnce(Return(std::optional<LayerFE::LayerSettings>())); + EXPECT_CALL(*mLayers[2].mLayerFE, prepareClientComposition(Eq(ByRef(layer2TargetSettings)))) + .WillOnce(Return(std::optional<LayerFE::LayerSettings>())); static_cast<void>( mOutput.generateClientCompositionRequestsHelper(false /* supportsProtectedContent */, @@ -4728,12 +4719,12 @@ TEST_F(GenerateClientCompositionRequestsTest_ThreeLayers, kLayerWhitePointNits, }; - EXPECT_CALL(*mLayers[0].mLayerFE, prepareClientCompositionList(Eq(ByRef(layer0TargetSettings)))) - .WillOnce(Return(std::vector<LayerFE::LayerSettings>())); - EXPECT_CALL(*mLayers[1].mLayerFE, prepareClientCompositionList(Eq(ByRef(layer1TargetSettings)))) - .WillOnce(Return(std::vector<LayerFE::LayerSettings>())); - EXPECT_CALL(*mLayers[2].mLayerFE, prepareClientCompositionList(Eq(ByRef(layer2TargetSettings)))) - .WillOnce(Return(std::vector<LayerFE::LayerSettings>())); + EXPECT_CALL(*mLayers[0].mLayerFE, prepareClientComposition(Eq(ByRef(layer0TargetSettings)))) + .WillOnce(Return(std::optional<LayerFE::LayerSettings>())); + EXPECT_CALL(*mLayers[1].mLayerFE, prepareClientComposition(Eq(ByRef(layer1TargetSettings)))) + .WillOnce(Return(std::optional<LayerFE::LayerSettings>())); + EXPECT_CALL(*mLayers[2].mLayerFE, prepareClientComposition(Eq(ByRef(layer2TargetSettings)))) + .WillOnce(Return(std::optional<LayerFE::LayerSettings>())); static_cast<void>( mOutput.generateClientCompositionRequestsHelper(false /* supportsProtectedContent */, @@ -4781,12 +4772,12 @@ TEST_F(GenerateClientCompositionRequestsTest_ThreeLayers, kLayerWhitePointNits, }; - EXPECT_CALL(*mLayers[0].mLayerFE, prepareClientCompositionList(Eq(ByRef(layer0TargetSettings)))) - .WillOnce(Return(std::vector<LayerFE::LayerSettings>())); - EXPECT_CALL(*mLayers[1].mLayerFE, prepareClientCompositionList(Eq(ByRef(layer1TargetSettings)))) - .WillOnce(Return(std::vector<LayerFE::LayerSettings>())); - EXPECT_CALL(*mLayers[2].mLayerFE, prepareClientCompositionList(Eq(ByRef(layer2TargetSettings)))) - .WillOnce(Return(std::vector<LayerFE::LayerSettings>())); + EXPECT_CALL(*mLayers[0].mLayerFE, prepareClientComposition(Eq(ByRef(layer0TargetSettings)))) + .WillOnce(Return(std::optional<LayerFE::LayerSettings>())); + EXPECT_CALL(*mLayers[1].mLayerFE, prepareClientComposition(Eq(ByRef(layer1TargetSettings)))) + .WillOnce(Return(std::optional<LayerFE::LayerSettings>())); + EXPECT_CALL(*mLayers[2].mLayerFE, prepareClientComposition(Eq(ByRef(layer2TargetSettings)))) + .WillOnce(Return(std::optional<LayerFE::LayerSettings>())); static_cast<void>( mOutput.generateClientCompositionRequestsHelper(false /* supportsProtectedContent */, @@ -4832,12 +4823,12 @@ TEST_F(GenerateClientCompositionRequestsTest_ThreeLayers, kLayerWhitePointNits, }; - EXPECT_CALL(*mLayers[0].mLayerFE, prepareClientCompositionList(Eq(ByRef(layer0TargetSettings)))) - .WillOnce(Return(std::vector<LayerFE::LayerSettings>())); - EXPECT_CALL(*mLayers[1].mLayerFE, prepareClientCompositionList(Eq(ByRef(layer1TargetSettings)))) - .WillOnce(Return(std::vector<LayerFE::LayerSettings>())); - EXPECT_CALL(*mLayers[2].mLayerFE, prepareClientCompositionList(Eq(ByRef(layer2TargetSettings)))) - .WillOnce(Return(std::vector<LayerFE::LayerSettings>())); + EXPECT_CALL(*mLayers[0].mLayerFE, prepareClientComposition(Eq(ByRef(layer0TargetSettings)))) + .WillOnce(Return(std::optional<LayerFE::LayerSettings>())); + EXPECT_CALL(*mLayers[1].mLayerFE, prepareClientComposition(Eq(ByRef(layer1TargetSettings)))) + .WillOnce(Return(std::optional<LayerFE::LayerSettings>())); + EXPECT_CALL(*mLayers[2].mLayerFE, prepareClientComposition(Eq(ByRef(layer2TargetSettings)))) + .WillOnce(Return(std::optional<LayerFE::LayerSettings>())); static_cast<void>(mOutput.generateClientCompositionRequestsHelper(true /* supportsProtectedContent */, kDisplayDataspace)); @@ -5018,8 +5009,8 @@ TEST_F(GenerateClientCompositionRequestsTest, handlesLandscapeModeSplitScreenReq EXPECT_CALL(leftLayer.mOutputLayer, requiresClientComposition()).WillRepeatedly(Return(true)); EXPECT_CALL(leftLayer.mOutputLayer, needsFiltering()).WillRepeatedly(Return(false)); - EXPECT_CALL(*leftLayer.mLayerFE, prepareClientCompositionList(Eq(ByRef(leftLayerSettings)))) - .WillOnce(Return(std::vector<LayerFE::LayerSettings>({leftLayer.mLayerSettings}))); + EXPECT_CALL(*leftLayer.mLayerFE, prepareClientComposition(Eq(ByRef(leftLayerSettings)))) + .WillOnce(Return(std::optional<LayerFE::LayerSettings>(leftLayer.mLayerSettings))); compositionengine::LayerFE::ClientCompositionTargetSettings rightLayerSettings{ Region(Rect(1000, 0, 2000, 1000)), @@ -5036,8 +5027,8 @@ TEST_F(GenerateClientCompositionRequestsTest, handlesLandscapeModeSplitScreenReq EXPECT_CALL(rightLayer.mOutputLayer, requiresClientComposition()).WillRepeatedly(Return(true)); EXPECT_CALL(rightLayer.mOutputLayer, needsFiltering()).WillRepeatedly(Return(false)); - EXPECT_CALL(*rightLayer.mLayerFE, prepareClientCompositionList(Eq(ByRef(rightLayerSettings)))) - .WillOnce(Return(std::vector<LayerFE::LayerSettings>({rightLayer.mLayerSettings}))); + EXPECT_CALL(*rightLayer.mLayerFE, prepareClientComposition(Eq(ByRef(rightLayerSettings)))) + .WillOnce(Return(std::optional<LayerFE::LayerSettings>(rightLayer.mLayerSettings))); constexpr bool supportsProtectedContent = true; auto requests = @@ -5075,8 +5066,8 @@ TEST_F(GenerateClientCompositionRequestsTest_ThreeLayers, EXPECT_CALL(mLayers[0].mOutputLayer, requiresClientComposition()).WillOnce(Return(false)); EXPECT_CALL(mLayers[1].mOutputLayer, requiresClientComposition()).WillOnce(Return(false)); - EXPECT_CALL(*mLayers[2].mLayerFE, prepareClientCompositionList(Eq(ByRef(layer2Settings)))) - .WillOnce(Return(std::vector<LayerFE::LayerSettings>({mShadowSettings}))); + EXPECT_CALL(*mLayers[2].mLayerFE, prepareClientComposition(Eq(ByRef(layer2Settings)))) + .WillOnce(Return(std::optional<LayerFE::LayerSettings>(mShadowSettings))); auto requests = mOutput.generateClientCompositionRequestsHelper(false /* supportsProtectedContent */, kDisplayDataspace); @@ -5093,9 +5084,6 @@ TEST_F(GenerateClientCompositionRequestsTest_ThreeLayers, const Region kPartialContentWithPartialShadowRegion = Region(kContentWithShadow).subtract(Rect(40, 40, 50, 80)); - LayerFE::LayerSettings mShadowSettings; - mShadowSettings.source.solidColor = {0.1f, 0.1f, 0.1f}; - mLayers[2].mOutputLayerState.visibleRegion = kPartialContentWithPartialShadowRegion; mLayers[2].mOutputLayerState.shadowRegion = kShadowRegion; @@ -5114,16 +5102,14 @@ TEST_F(GenerateClientCompositionRequestsTest_ThreeLayers, EXPECT_CALL(mLayers[0].mOutputLayer, requiresClientComposition()).WillOnce(Return(false)); EXPECT_CALL(mLayers[1].mOutputLayer, requiresClientComposition()).WillOnce(Return(false)); - EXPECT_CALL(*mLayers[2].mLayerFE, prepareClientCompositionList(Eq(ByRef(layer2Settings)))) - .WillOnce(Return(std::vector<LayerFE::LayerSettings>( - {mShadowSettings, mLayers[2].mLayerSettings}))); + EXPECT_CALL(*mLayers[2].mLayerFE, prepareClientComposition(Eq(ByRef(layer2Settings)))) + .WillOnce(Return(std::optional<LayerFE::LayerSettings>(mLayers[2].mLayerSettings))); auto requests = mOutput.generateClientCompositionRequestsHelper(false /* supportsProtectedContent */, kDisplayDataspace); - ASSERT_EQ(2u, requests.size()); + ASSERT_EQ(1u, requests.size()); - EXPECT_EQ(mShadowSettings, requests[0]); - EXPECT_EQ(mLayers[2].mLayerSettings, requests[1]); + EXPECT_EQ(mLayers[2].mLayerSettings, requests[0]); } } // namespace diff --git a/services/surfaceflinger/CompositionEngine/tests/planner/CachedSetTest.cpp b/services/surfaceflinger/CompositionEngine/tests/planner/CachedSetTest.cpp index 8a99e4e2e8..cb4c4e23fe 100644 --- a/services/surfaceflinger/CompositionEngine/tests/planner/CachedSetTest.cpp +++ b/services/surfaceflinger/CompositionEngine/tests/planner/CachedSetTest.cpp @@ -345,13 +345,13 @@ TEST_F(CachedSetTest, renderUnsecureOutput) { CachedSet cachedSet(layer1); cachedSet.append(CachedSet(layer2)); - std::vector<compositionengine::LayerFE::LayerSettings> clientCompList1; - clientCompList1.push_back({}); - clientCompList1[0].alpha = 0.5f; + std::optional<compositionengine::LayerFE::LayerSettings> clientComp1; + clientComp1.emplace(); + clientComp1->alpha = 0.5f; - std::vector<compositionengine::LayerFE::LayerSettings> clientCompList2; - clientCompList2.push_back({}); - clientCompList2[0].alpha = 0.75f; + std::optional<compositionengine::LayerFE::LayerSettings> clientComp2; + clientComp2.emplace(); + clientComp2->alpha = 0.75f; const auto drawLayers = [&](const renderengine::DisplaySettings& displaySettings, @@ -368,12 +368,10 @@ TEST_F(CachedSetTest, renderUnsecureOutput) { return futureOf<renderengine::RenderEngineResult>({NO_ERROR, base::unique_fd()}); }; - EXPECT_CALL(*layerFE1, - prepareClientCompositionList(ClientCompositionTargetSettingsSecureEq(false))) - .WillOnce(Return(clientCompList1)); - EXPECT_CALL(*layerFE2, - prepareClientCompositionList(ClientCompositionTargetSettingsSecureEq(false))) - .WillOnce(Return(clientCompList2)); + EXPECT_CALL(*layerFE1, prepareClientComposition(ClientCompositionTargetSettingsSecureEq(false))) + .WillOnce(Return(clientComp1)); + EXPECT_CALL(*layerFE2, prepareClientComposition(ClientCompositionTargetSettingsSecureEq(false))) + .WillOnce(Return(clientComp2)); EXPECT_CALL(mRenderEngine, drawLayers(_, _, _, _, _)).WillOnce(Invoke(drawLayers)); mOutputState.isSecure = false; cachedSet.render(mRenderEngine, mTexturePool, mOutputState); @@ -397,13 +395,13 @@ TEST_F(CachedSetTest, renderSecureOutput) { CachedSet cachedSet(layer1); cachedSet.append(CachedSet(layer2)); - std::vector<compositionengine::LayerFE::LayerSettings> clientCompList1; - clientCompList1.push_back({}); - clientCompList1[0].alpha = 0.5f; + std::optional<compositionengine::LayerFE::LayerSettings> clientComp1; + clientComp1.emplace(); + clientComp1->alpha = 0.5f; - std::vector<compositionengine::LayerFE::LayerSettings> clientCompList2; - clientCompList2.push_back({}); - clientCompList2[0].alpha = 0.75f; + std::optional<compositionengine::LayerFE::LayerSettings> clientComp2; + clientComp2.emplace(); + clientComp2->alpha = 0.75f; const auto drawLayers = [&](const renderengine::DisplaySettings& displaySettings, @@ -421,12 +419,10 @@ TEST_F(CachedSetTest, renderSecureOutput) { return futureOf<renderengine::RenderEngineResult>({NO_ERROR, base::unique_fd()}); }; - EXPECT_CALL(*layerFE1, - prepareClientCompositionList(ClientCompositionTargetSettingsSecureEq(true))) - .WillOnce(Return(clientCompList1)); - EXPECT_CALL(*layerFE2, - prepareClientCompositionList(ClientCompositionTargetSettingsSecureEq(true))) - .WillOnce(Return(clientCompList2)); + EXPECT_CALL(*layerFE1, prepareClientComposition(ClientCompositionTargetSettingsSecureEq(true))) + .WillOnce(Return(clientComp1)); + EXPECT_CALL(*layerFE2, prepareClientComposition(ClientCompositionTargetSettingsSecureEq(true))) + .WillOnce(Return(clientComp2)); EXPECT_CALL(mRenderEngine, drawLayers(_, _, _, _, _)).WillOnce(Invoke(drawLayers)); mOutputState.isSecure = true; cachedSet.render(mRenderEngine, mTexturePool, mOutputState); @@ -450,11 +446,11 @@ TEST_F(CachedSetTest, renderWhitePoint) { CachedSet cachedSet(layer1); cachedSet.append(CachedSet(layer2)); - std::vector<compositionengine::LayerFE::LayerSettings> clientCompList1; - clientCompList1.push_back({}); + std::optional<compositionengine::LayerFE::LayerSettings> clientComp1; + clientComp1.emplace(); - std::vector<compositionengine::LayerFE::LayerSettings> clientCompList2; - clientCompList2.push_back({}); + std::optional<compositionengine::LayerFE::LayerSettings> clientComp2; + clientComp2.emplace(); mOutputState.displayBrightnessNits = 400.f; @@ -468,13 +464,13 @@ TEST_F(CachedSetTest, renderWhitePoint) { }; EXPECT_CALL(*layerFE1, - prepareClientCompositionList(ClientCompositionTargetSettingsWhitePointEq( + prepareClientComposition(ClientCompositionTargetSettingsWhitePointEq( mOutputState.displayBrightnessNits))) - .WillOnce(Return(clientCompList1)); + .WillOnce(Return(clientComp1)); EXPECT_CALL(*layerFE2, - prepareClientCompositionList(ClientCompositionTargetSettingsWhitePointEq( + prepareClientComposition(ClientCompositionTargetSettingsWhitePointEq( mOutputState.displayBrightnessNits))) - .WillOnce(Return(clientCompList2)); + .WillOnce(Return(clientComp2)); EXPECT_CALL(mRenderEngine, drawLayers(_, _, _, _, _)).WillOnce(Invoke(drawLayers)); mOutputState.isSecure = true; cachedSet.render(mRenderEngine, mTexturePool, mOutputState); @@ -498,13 +494,13 @@ TEST_F(CachedSetTest, rendersWithOffsetFramebufferContent) { CachedSet cachedSet(layer1); cachedSet.append(CachedSet(layer2)); - std::vector<compositionengine::LayerFE::LayerSettings> clientCompList1; - clientCompList1.push_back({}); - clientCompList1[0].alpha = 0.5f; + std::optional<compositionengine::LayerFE::LayerSettings> clientComp1; + clientComp1.emplace(); + clientComp1->alpha = 0.5f; - std::vector<compositionengine::LayerFE::LayerSettings> clientCompList2; - clientCompList2.push_back({}); - clientCompList2[0].alpha = 0.75f; + std::optional<compositionengine::LayerFE::LayerSettings> clientComp2; + clientComp2.emplace(); + clientComp2->alpha = 0.75f; mOutputState.framebufferSpace = ProjectionSpace(ui::Size(10, 20), Rect(2, 3, 10, 5)); @@ -524,8 +520,8 @@ TEST_F(CachedSetTest, rendersWithOffsetFramebufferContent) { return futureOf<renderengine::RenderEngineResult>({NO_ERROR, base::unique_fd()}); }; - EXPECT_CALL(*layerFE1, prepareClientCompositionList(_)).WillOnce(Return(clientCompList1)); - EXPECT_CALL(*layerFE2, prepareClientCompositionList(_)).WillOnce(Return(clientCompList2)); + EXPECT_CALL(*layerFE1, prepareClientComposition(_)).WillOnce(Return(clientComp1)); + EXPECT_CALL(*layerFE2, prepareClientComposition(_)).WillOnce(Return(clientComp2)); EXPECT_CALL(mRenderEngine, drawLayers(_, _, _, _, _)).WillOnce(Invoke(drawLayers)); cachedSet.render(mRenderEngine, mTexturePool, mOutputState); expectReadyBuffer(cachedSet); @@ -722,22 +718,22 @@ TEST_F(CachedSetTest, addHolePunch) { cachedSet.addHolePunchLayerIfFeasible(layer3, true); - std::vector<compositionengine::LayerFE::LayerSettings> clientCompList1; - clientCompList1.push_back({}); - std::vector<compositionengine::LayerFE::LayerSettings> clientCompList2; - clientCompList2.push_back({}); - std::vector<compositionengine::LayerFE::LayerSettings> clientCompList3; - clientCompList3.push_back({}); + std::optional<compositionengine::LayerFE::LayerSettings> clientComp1; + clientComp1.emplace(); + std::optional<compositionengine::LayerFE::LayerSettings> clientComp2; + clientComp2.emplace(); + std::optional<compositionengine::LayerFE::LayerSettings> clientComp3; + clientComp3.emplace(); - clientCompList3[0].source.buffer.buffer = + clientComp3->source.buffer.buffer = std::make_shared<renderengine::mock::FakeExternalTexture>(1U /*width*/, 1U /*height*/, 1ULL /* bufferId */, HAL_PIXEL_FORMAT_RGBA_8888, 0ULL /*usage*/); - EXPECT_CALL(*layerFE1, prepareClientCompositionList(_)).WillOnce(Return(clientCompList1)); - EXPECT_CALL(*layerFE2, prepareClientCompositionList(_)).WillOnce(Return(clientCompList2)); - EXPECT_CALL(*layerFE3, prepareClientCompositionList(_)).WillOnce(Return(clientCompList3)); + EXPECT_CALL(*layerFE1, prepareClientComposition(_)).WillOnce(Return(clientComp1)); + EXPECT_CALL(*layerFE2, prepareClientComposition(_)).WillOnce(Return(clientComp2)); + EXPECT_CALL(*layerFE3, prepareClientComposition(_)).WillOnce(Return(clientComp3)); const auto drawLayers = [&](const renderengine::DisplaySettings&, @@ -771,7 +767,7 @@ TEST_F(CachedSetTest, addHolePunch) { } TEST_F(CachedSetTest, addHolePunch_noBuffer) { - // Same as addHolePunch, except that clientCompList3 does not contain a + // Same as addHolePunch, except that clientComp3 does not contain a // buffer. This imitates the case where the buffer had protected content, so // BufferLayer did not add it to the LayerSettings. This should not assert. mTestLayers[0]->outputLayerCompositionState.displayFrame = Rect(0, 0, 5, 5); @@ -789,16 +785,16 @@ TEST_F(CachedSetTest, addHolePunch_noBuffer) { cachedSet.addHolePunchLayerIfFeasible(layer3, true); - std::vector<compositionengine::LayerFE::LayerSettings> clientCompList1; - clientCompList1.push_back({}); - std::vector<compositionengine::LayerFE::LayerSettings> clientCompList2; - clientCompList2.push_back({}); - std::vector<compositionengine::LayerFE::LayerSettings> clientCompList3; - clientCompList3.push_back({}); + std::optional<compositionengine::LayerFE::LayerSettings> clientComp1; + clientComp1.emplace(); + std::optional<compositionengine::LayerFE::LayerSettings> clientComp2; + clientComp2.emplace(); + std::optional<compositionengine::LayerFE::LayerSettings> clientComp3; + clientComp3.emplace(); - EXPECT_CALL(*layerFE1, prepareClientCompositionList(_)).WillOnce(Return(clientCompList1)); - EXPECT_CALL(*layerFE2, prepareClientCompositionList(_)).WillOnce(Return(clientCompList2)); - EXPECT_CALL(*layerFE3, prepareClientCompositionList(_)).WillOnce(Return(clientCompList3)); + EXPECT_CALL(*layerFE1, prepareClientComposition(_)).WillOnce(Return(clientComp1)); + EXPECT_CALL(*layerFE2, prepareClientComposition(_)).WillOnce(Return(clientComp2)); + EXPECT_CALL(*layerFE3, prepareClientComposition(_)).WillOnce(Return(clientComp3)); const auto drawLayers = [&](const renderengine::DisplaySettings&, @@ -923,34 +919,34 @@ TEST_F(CachedSetTest, addBlur) { cachedSet.addBackgroundBlurLayer(layer3); - std::vector<compositionengine::LayerFE::LayerSettings> clientCompList1; - clientCompList1.push_back({}); - std::vector<compositionengine::LayerFE::LayerSettings> clientCompList2; - clientCompList2.push_back({}); - std::vector<compositionengine::LayerFE::LayerSettings> clientCompList3; - clientCompList3.push_back({}); + std::optional<compositionengine::LayerFE::LayerSettings> clientComp1; + clientComp1.emplace(); + std::optional<compositionengine::LayerFE::LayerSettings> clientComp2; + clientComp2.emplace(); + std::optional<compositionengine::LayerFE::LayerSettings> clientComp3; + clientComp3.emplace(); - clientCompList3[0].source.buffer.buffer = + clientComp3->source.buffer.buffer = std::make_shared<renderengine::mock::FakeExternalTexture>(1U /*width*/, 1U /*height*/, 1ULL /* bufferId */, HAL_PIXEL_FORMAT_RGBA_8888, 0ULL /*usage*/); EXPECT_CALL(*layerFE1, - prepareClientCompositionList(ClientCompositionTargetSettingsBlurSettingsEq( + prepareClientComposition(ClientCompositionTargetSettingsBlurSettingsEq( compositionengine::LayerFE::ClientCompositionTargetSettings::BlurSetting:: Enabled))) - .WillOnce(Return(clientCompList1)); + .WillOnce(Return(clientComp1)); EXPECT_CALL(*layerFE2, - prepareClientCompositionList(ClientCompositionTargetSettingsBlurSettingsEq( + prepareClientComposition(ClientCompositionTargetSettingsBlurSettingsEq( compositionengine::LayerFE::ClientCompositionTargetSettings::BlurSetting:: Enabled))) - .WillOnce(Return(clientCompList2)); + .WillOnce(Return(clientComp2)); EXPECT_CALL(*layerFE3, - prepareClientCompositionList(ClientCompositionTargetSettingsBlurSettingsEq( + prepareClientComposition(ClientCompositionTargetSettingsBlurSettingsEq( compositionengine::LayerFE::ClientCompositionTargetSettings::BlurSetting:: BackgroundBlurOnly))) - .WillOnce(Return(clientCompList3)); + .WillOnce(Return(clientComp3)); const auto drawLayers = [&](const renderengine::DisplaySettings&, diff --git a/services/surfaceflinger/CompositionEngine/tests/planner/FlattenerTest.cpp b/services/surfaceflinger/CompositionEngine/tests/planner/FlattenerTest.cpp index 200278cb84..b624d1a2ea 100644 --- a/services/surfaceflinger/CompositionEngine/tests/planner/FlattenerTest.cpp +++ b/services/surfaceflinger/CompositionEngine/tests/planner/FlattenerTest.cpp @@ -124,12 +124,11 @@ void FlattenerTest::SetUp() { EXPECT_CALL(*testLayer->layerFE, getCompositionState) .WillRepeatedly(Return(&testLayer->layerFECompositionState)); - std::vector<LayerFE::LayerSettings> clientCompositionList = { - LayerFE::LayerSettings{}, - }; + std::optional<LayerFE::LayerSettings> clientComposition; + clientComposition.emplace(); - EXPECT_CALL(*testLayer->layerFE, prepareClientCompositionList) - .WillRepeatedly(Return(clientCompositionList)); + EXPECT_CALL(*testLayer->layerFE, prepareClientComposition) + .WillRepeatedly(Return(clientComposition)); EXPECT_CALL(testLayer->outputLayer, getLayerFE) .WillRepeatedly(ReturnRef(*testLayer->layerFE)); EXPECT_CALL(testLayer->outputLayer, getState) @@ -638,16 +637,15 @@ TEST_F(FlattenerTest, flattenLayers_pip) { EXPECT_CALL(*mTestLayers[2]->layerFE, hasRoundedCorners()).WillRepeatedly(Return(true)); - std::vector<LayerFE::LayerSettings> clientCompositionList = { - LayerFE::LayerSettings{}, - }; - clientCompositionList[0].source.buffer.buffer = std::make_shared< + std::optional<LayerFE::LayerSettings> clientComposition; + clientComposition.emplace(); + clientComposition->source.buffer.buffer = std::make_shared< renderengine::impl::ExternalTexture>(mTestLayers[2]->layerFECompositionState.buffer, mRenderEngine, renderengine::impl::ExternalTexture::Usage:: READABLE); - EXPECT_CALL(*mTestLayers[2]->layerFE, prepareClientCompositionList(_)) - .WillOnce(Return(clientCompositionList)); + EXPECT_CALL(*mTestLayers[2]->layerFE, prepareClientComposition(_)) + .WillOnce(Return(clientComposition)); const std::vector<const LayerState*> layers = { layerState1.get(), @@ -712,16 +710,15 @@ TEST_F(FlattenerTest, flattenLayers_holePunchSingleLayer) { EXPECT_CALL(*mTestLayers[1]->layerFE, hasRoundedCorners()).WillRepeatedly(Return(true)); - std::vector<LayerFE::LayerSettings> clientCompositionList = { - LayerFE::LayerSettings{}, - }; - clientCompositionList[0].source.buffer.buffer = std::make_shared< + std::optional<LayerFE::LayerSettings> clientComposition; + clientComposition.emplace(); + clientComposition->source.buffer.buffer = std::make_shared< renderengine::impl::ExternalTexture>(mTestLayers[1]->layerFECompositionState.buffer, mRenderEngine, renderengine::impl::ExternalTexture::Usage:: READABLE); - EXPECT_CALL(*mTestLayers[1]->layerFE, prepareClientCompositionList(_)) - .WillOnce(Return(clientCompositionList)); + EXPECT_CALL(*mTestLayers[1]->layerFE, prepareClientComposition(_)) + .WillOnce(Return(clientComposition)); const std::vector<const LayerState*> layers = { layerState0.get(), @@ -784,16 +781,15 @@ TEST_F(FlattenerTest, flattenLayers_holePunchSingleColorLayer) { EXPECT_CALL(*mTestLayers[1]->layerFE, hasRoundedCorners()).WillRepeatedly(Return(true)); - std::vector<LayerFE::LayerSettings> clientCompositionList = { - LayerFE::LayerSettings{}, - }; - clientCompositionList[0].source.buffer.buffer = std::make_shared< + std::optional<LayerFE::LayerSettings> clientComposition; + clientComposition.emplace(); + clientComposition->source.buffer.buffer = std::make_shared< renderengine::impl::ExternalTexture>(mTestLayers[1]->layerFECompositionState.buffer, mRenderEngine, renderengine::impl::ExternalTexture::Usage:: READABLE); - EXPECT_CALL(*mTestLayers[1]->layerFE, prepareClientCompositionList(_)) - .WillOnce(Return(clientCompositionList)); + EXPECT_CALL(*mTestLayers[1]->layerFE, prepareClientComposition(_)) + .WillOnce(Return(clientComposition)); const std::vector<const LayerState*> layers = { layerState0.get(), diff --git a/services/surfaceflinger/EffectLayer.cpp b/services/surfaceflinger/EffectLayer.cpp index d8bbc306e4..d161c510fd 100644 --- a/services/surfaceflinger/EffectLayer.cpp +++ b/services/surfaceflinger/EffectLayer.cpp @@ -47,11 +47,10 @@ EffectLayer::EffectLayer(const LayerCreationArgs& args) EffectLayer::~EffectLayer() = default; -std::vector<compositionengine::LayerFE::LayerSettings> EffectLayer::prepareClientCompositionList( - compositionengine::LayerFE::ClientCompositionTargetSettings& targetSettings) { - std::vector<compositionengine::LayerFE::LayerSettings> results; +std::optional<compositionengine::LayerFE::LayerSettings> EffectLayer::prepareClientComposition( + compositionengine::LayerFE::ClientCompositionTargetSettings& targetSettings) const { std::optional<compositionengine::LayerFE::LayerSettings> layerSettings = - prepareClientComposition(targetSettings); + Layer::prepareClientComposition(targetSettings); // Nothing to render. if (!layerSettings) { return {}; @@ -64,13 +63,13 @@ std::vector<compositionengine::LayerFE::LayerSettings> EffectLayer::prepareClien if (targetSettings.realContentIsVisible && fillsColor()) { // Set color for color fill settings. layerSettings->source.solidColor = getColor().rgb; - results.push_back(*layerSettings); + return layerSettings; } else if (hasBlur() || drawShadows()) { layerSettings->skipContentDraw = true; - results.push_back(*layerSettings); + return layerSettings; } - return results; + return {}; } bool EffectLayer::isVisible() const { diff --git a/services/surfaceflinger/EffectLayer.h b/services/surfaceflinger/EffectLayer.h index 1dcb633251..747b0bfe9a 100644 --- a/services/surfaceflinger/EffectLayer.h +++ b/services/surfaceflinger/EffectLayer.h @@ -52,8 +52,9 @@ protected: */ const compositionengine::LayerFECompositionState* getCompositionState() const override; void preparePerFrameCompositionState() override; - std::vector<compositionengine::LayerFE::LayerSettings> prepareClientCompositionList( - compositionengine::LayerFE::ClientCompositionTargetSettings& targetSettings) override; + std::optional<compositionengine::LayerFE::LayerSettings> prepareClientComposition( + compositionengine::LayerFE::ClientCompositionTargetSettings& targetSettings) + const override; std::unique_ptr<compositionengine::LayerFECompositionState> mCompositionState; diff --git a/services/surfaceflinger/Layer.cpp b/services/surfaceflinger/Layer.cpp index 5e1a858d0e..dfff8fe8ee 100644 --- a/services/surfaceflinger/Layer.cpp +++ b/services/surfaceflinger/Layer.cpp @@ -575,7 +575,7 @@ const char* Layer::getDebugName() const { // --------------------------------------------------------------------------- std::optional<compositionengine::LayerFE::LayerSettings> Layer::prepareClientComposition( - compositionengine::LayerFE::ClientCompositionTargetSettings& targetSettings) { + compositionengine::LayerFE::ClientCompositionTargetSettings& targetSettings) const { if (!getCompositionState()) { return {}; } @@ -653,30 +653,6 @@ void Layer::prepareClearClientComposition(LayerFE::LayerSettings& layerSettings, layerSettings.name = getName(); } -// TODO(b/188891810): This method now only ever returns 0 or 1 layers so we should return -// std::optional instead of a vector. Additionally, we should consider removing -// this method entirely in favor of calling prepareClientComposition directly. -std::vector<compositionengine::LayerFE::LayerSettings> Layer::prepareClientCompositionList( - compositionengine::LayerFE::ClientCompositionTargetSettings& targetSettings) { - std::optional<compositionengine::LayerFE::LayerSettings> layerSettings = - prepareClientComposition(targetSettings); - // Nothing to render. - if (!layerSettings) { - return {}; - } - - // HWC requests to clear this layer. - if (targetSettings.clearContent) { - prepareClearClientComposition(*layerSettings, false /* blackout */); - return {*layerSettings}; - } - - // set the shadow for the layer if needed - prepareShadowClientComposition(*layerSettings, targetSettings.viewport); - - return {*layerSettings}; -} - aidl::android::hardware::graphics::composer3::Composition Layer::getCompositionType( const DisplayDevice& display) const { const auto outputLayer = findOutputLayerForDisplay(&display); @@ -2006,7 +1982,7 @@ Layer::RoundedCornerState Layer::getRoundedCornerState() const { } void Layer::prepareShadowClientComposition(LayerFE::LayerSettings& caster, - const Rect& layerStackRect) { + const Rect& layerStackRect) const { renderengine::ShadowSettings state = mFlinger->mDrawingState.globalShadowSettings; // Note: this preserves existing behavior of shadowing the entire layer and not cropping it if diff --git a/services/surfaceflinger/Layer.h b/services/surfaceflinger/Layer.h index 22bb866368..6e83b235d4 100644 --- a/services/surfaceflinger/Layer.h +++ b/services/surfaceflinger/Layer.h @@ -618,8 +618,9 @@ public: const compositionengine::LayerFECompositionState* getCompositionState() const override; bool onPreComposition(nsecs_t) override; void prepareCompositionState(compositionengine::LayerFE::StateSubset subset) override; - std::vector<compositionengine::LayerFE::LayerSettings> prepareClientCompositionList( - compositionengine::LayerFE::ClientCompositionTargetSettings&) override; + + std::optional<compositionengine::LayerFE::LayerSettings> prepareClientComposition( + compositionengine::LayerFE::ClientCompositionTargetSettings&) const override; void onLayerDisplayed(ftl::SharedFuture<FenceResult>) override; void setWasClientComposed(const sp<Fence>& fence) override { @@ -918,8 +919,6 @@ protected: friend class TransactionSurfaceFrameTest; virtual void setInitialValuesForClone(const sp<Layer>& clonedFrom); - virtual std::optional<compositionengine::LayerFE::LayerSettings> prepareClientComposition( - compositionengine::LayerFE::ClientCompositionTargetSettings&); virtual void preparePerFrameCompositionState(); virtual void commitTransaction(State& stateToCommit); virtual void onSurfaceFrameCreated(const std::shared_ptr<frametimeline::SurfaceFrame>&) {} @@ -940,7 +939,8 @@ protected: // Modifies the passed in layer settings to clear the contents. If the blackout flag is set, // the settings clears the content with a solid black fill. void prepareClearClientComposition(LayerFE::LayerSettings&, bool blackout) const; - void prepareShadowClientComposition(LayerFE::LayerSettings& caster, const Rect& layerStackRect); + void prepareShadowClientComposition(LayerFE::LayerSettings& caster, + const Rect& layerStackRect) const; void prepareBasicGeometryCompositionState(); void prepareGeometryCompositionState(); diff --git a/services/surfaceflinger/Scheduler/MessageQueue.cpp b/services/surfaceflinger/Scheduler/MessageQueue.cpp index e1ac301622..e4e65b4828 100644 --- a/services/surfaceflinger/Scheduler/MessageQueue.cpp +++ b/services/surfaceflinger/Scheduler/MessageQueue.cpp @@ -158,6 +158,19 @@ void MessageQueue::postMessage(sp<MessageHandler>&& handler) { mLooper->sendMessage(handler, Message()); } +void MessageQueue::scheduleConfigure() { + struct ConfigureHandler : MessageHandler { + explicit ConfigureHandler(ICompositor& compositor) : compositor(compositor) {} + + void handleMessage(const Message&) override { compositor.configure(); } + + ICompositor& compositor; + }; + + // TODO(b/241285876): Batch configure tasks that happen within some duration. + postMessage(sp<ConfigureHandler>::make(mCompositor)); +} + void MessageQueue::scheduleFrame() { ATRACE_CALL(); diff --git a/services/surfaceflinger/Scheduler/MessageQueue.h b/services/surfaceflinger/Scheduler/MessageQueue.h index 506f27b460..899233a151 100644 --- a/services/surfaceflinger/Scheduler/MessageQueue.h +++ b/services/surfaceflinger/Scheduler/MessageQueue.h @@ -38,6 +38,7 @@ namespace android { struct ICompositor { + virtual void configure() = 0; virtual bool commit(TimePoint frameTime, VsyncId, TimePoint expectedVsyncTime) = 0; virtual void composite(TimePoint frameTime, VsyncId) = 0; virtual void sample() = 0; @@ -78,6 +79,7 @@ public: virtual void setInjector(sp<EventThreadConnection>) = 0; virtual void waitMessage() = 0; virtual void postMessage(sp<MessageHandler>&&) = 0; + virtual void scheduleConfigure() = 0; virtual void scheduleFrame() = 0; using Clock = std::chrono::steady_clock; @@ -152,6 +154,7 @@ public: void waitMessage() override; void postMessage(sp<MessageHandler>&&) override; + void scheduleConfigure() override; void scheduleFrame() override; std::optional<Clock::time_point> getScheduledFrameTime() const override; diff --git a/services/surfaceflinger/Scheduler/Scheduler.h b/services/surfaceflinger/Scheduler/Scheduler.h index 47304932f1..eb3856d6fc 100644 --- a/services/surfaceflinger/Scheduler/Scheduler.h +++ b/services/surfaceflinger/Scheduler/Scheduler.h @@ -116,6 +116,7 @@ public: using Impl::getScheduledFrameTime; using Impl::setDuration; + using Impl::scheduleConfigure; using Impl::scheduleFrame; // Schedule an asynchronous or synchronous task on the main thread. diff --git a/services/surfaceflinger/SurfaceFlinger.cpp b/services/surfaceflinger/SurfaceFlinger.cpp index f7146c66cb..6b4cfa1249 100644 --- a/services/surfaceflinger/SurfaceFlinger.cpp +++ b/services/surfaceflinger/SurfaceFlinger.cpp @@ -763,10 +763,10 @@ chooseRenderEngineTypeViaSysProp() { // Do not call property_set on main thread which will be blocked by init // Use StartPropertySetThread instead. -void SurfaceFlinger::init() { +void SurfaceFlinger::init() FTL_FAKE_GUARD(kMainThreadContext) { ALOGI( "SurfaceFlinger's main thread ready to run. " "Initializing graphics H/W..."); - Mutex::Autolock _l(mStateLock); + Mutex::Autolock lock(mStateLock); // Get a RenderEngine for the given display / config (can't fail) // TODO(b/77156734): We need to stop casting and use HAL types when possible. @@ -806,12 +806,14 @@ void SurfaceFlinger::init() { } // Process any initial hotplug and resulting display changes. - processDisplayHotplugEventsLocked(); + LOG_ALWAYS_FATAL_IF(!configureLocked(), + "Initial display configuration failed: HWC did not hotplug"); + processDisplayChangesLocked(); + const auto display = getDefaultDisplayDeviceLocked(); - LOG_ALWAYS_FATAL_IF(!display, "Missing primary display after registering composer callback."); - const auto displayId = display->getPhysicalId(); - LOG_ALWAYS_FATAL_IF(!getHwComposer().isConnected(displayId), - "Primary display is disconnected."); + LOG_ALWAYS_FATAL_IF(!display, "Failed to configure the primary display"); + LOG_ALWAYS_FATAL_IF(!getHwComposer().isConnected(display->getPhysicalId()), + "Primary display is disconnected"); // initialize our drawing state mDrawingState = mCurrentState; @@ -1869,23 +1871,14 @@ void SurfaceFlinger::onComposerHalVsync(hal::HWDisplayId hwcDisplayId, int64_t t void SurfaceFlinger::onComposerHalHotplug(hal::HWDisplayId hwcDisplayId, hal::Connection connection) { - const bool connected = connection == hal::Connection::CONNECTED; - ALOGI("%s HAL display %" PRIu64, connected ? "Connecting" : "Disconnecting", hwcDisplayId); - - // Only lock if we're not on the main thread. This function is normally - // called on a hwbinder thread, but for the primary display it's called on - // the main thread with the state lock already held, so don't attempt to - // acquire it here. - ConditionalLock lock(mStateLock, std::this_thread::get_id() != mMainThreadId); - - mPendingHotplugEvents.emplace_back(HotplugEvent{hwcDisplayId, connection}); - - if (std::this_thread::get_id() == mMainThreadId) { - // Process all pending hot plug events immediately if we are on the main thread. - processDisplayHotplugEventsLocked(); + { + std::lock_guard<std::mutex> lock(mHotplugMutex); + mPendingHotplugEvents.push_back(HotplugEvent{hwcDisplayId, connection}); } - setTransactionFlags(eDisplayTransactionNeeded); + if (mScheduler) { + mScheduler->scheduleConfigure(); + } } void SurfaceFlinger::onComposerHalVsyncPeriodTimingChanged( @@ -1958,6 +1951,13 @@ TimePoint SurfaceFlinger::calculateExpectedPresentTime(TimePoint frameTime) cons return vsyncDeadline + schedule.period(); } +void SurfaceFlinger::configure() FTL_FAKE_GUARD(kMainThreadContext) { + Mutex::Autolock lock(mStateLock); + if (configureLocked()) { + setTransactionFlags(eDisplayTransactionNeeded); + } +} + bool SurfaceFlinger::commit(TimePoint frameTime, VsyncId vsyncId, TimePoint expectedVsyncTime) FTL_FAKE_GUARD(kMainThreadContext) { // The expectedVsyncTime, which was predicted when this frame was scheduled, is normally in the @@ -2642,8 +2642,14 @@ std::pair<DisplayModes, DisplayModePtr> SurfaceFlinger::loadDisplayModes( return {modes, activeMode}; } -void SurfaceFlinger::processDisplayHotplugEventsLocked() { - for (const auto& event : mPendingHotplugEvents) { +bool SurfaceFlinger::configureLocked() { + std::vector<HotplugEvent> events; + { + std::lock_guard<std::mutex> lock(mHotplugMutex); + events = std::move(mPendingHotplugEvents); + } + + for (const auto& event : events) { std::optional<DisplayIdentificationInfo> info = getHwComposer().onHotplug(event.hwcDisplayId, event.connection); @@ -2653,6 +2659,7 @@ void SurfaceFlinger::processDisplayHotplugEventsLocked() { const auto displayId = info->id; const auto token = mPhysicalDisplayTokens.get(displayId); + const char* log; if (event.connection == hal::Connection::CONNECTED) { auto [supportedModes, activeMode] = loadDisplayModes(displayId); @@ -2664,7 +2671,7 @@ void SurfaceFlinger::processDisplayHotplugEventsLocked() { } if (!token) { - ALOGV("Creating display %s", to_string(displayId).c_str()); + log = "Connecting"; DisplayDeviceState state; state.physical = {.id = displayId, @@ -2681,7 +2688,7 @@ void SurfaceFlinger::processDisplayHotplugEventsLocked() { mPhysicalDisplayTokens.try_emplace(displayId, std::move(token)); mInterceptor->saveDisplayCreation(state); } else { - ALOGV("Recreating display %s", to_string(displayId).c_str()); + log = "Reconnecting"; auto& state = mCurrentState.displays.editValueFor(token->get()); state.sequenceId = DisplayDeviceState{}.sequenceId; // Generate new sequenceId. @@ -2692,7 +2699,7 @@ void SurfaceFlinger::processDisplayHotplugEventsLocked() { } } } else { - ALOGV("Removing display %s", to_string(displayId).c_str()); + log = "Disconnecting"; if (const ssize_t index = mCurrentState.displays.indexOfKey(token->get()); index >= 0) { const DisplayDeviceState& state = mCurrentState.displays.valueAt(index); @@ -2703,15 +2710,14 @@ void SurfaceFlinger::processDisplayHotplugEventsLocked() { mPhysicalDisplayTokens.erase(displayId); } - processDisplayChangesLocked(); + ALOGI("%s display %s (HAL ID %" PRIu64 ")", log, to_string(displayId).c_str(), + event.hwcDisplayId); } - mPendingHotplugEvents.clear(); + return !events.empty(); } void SurfaceFlinger::dispatchDisplayHotplugEvent(PhysicalDisplayId displayId, bool connected) { - ALOGI("Dispatching display hotplug event displayId=%s, connected=%d", - to_string(displayId).c_str(), connected); mScheduler->onHotplugReceived(mAppConnectionHandle, displayId, connected); mScheduler->onHotplugReceived(mSfConnectionHandle, displayId, connected); } @@ -3041,7 +3047,6 @@ void SurfaceFlinger::commitTransactionsLocked(uint32_t transactionFlags) { const bool displayTransactionNeeded = transactionFlags & eDisplayTransactionNeeded; if (displayTransactionNeeded) { processDisplayChangesLocked(); - processDisplayHotplugEventsLocked(); } mForceTransactionDisplayChange = displayTransactionNeeded; @@ -6579,27 +6584,24 @@ ftl::SharedFuture<FenceResult> SurfaceFlinger::renderScreenImpl( isHdrLayer(layer) ? displayBrightnessNits : sdrWhitePointNits, }; - std::vector<compositionengine::LayerFE::LayerSettings> results = - layer->prepareClientCompositionList(targetSettings); - if (results.size() > 0) { - for (auto& settings : results) { - settings.geometry.positionTransform = - transform.asMatrix4() * settings.geometry.positionTransform; - // There's no need to process blurs when we're executing region sampling, - // we're just trying to understand what we're drawing, and doing so without - // blurs is already a pretty good approximation. - if (regionSampling) { - settings.backgroundBlurRadius = 0; - } - captureResults.capturedHdrLayers |= isHdrLayer(layer); - } + std::optional<compositionengine::LayerFE::LayerSettings> settings = + layer->prepareClientComposition(targetSettings); + if (!settings) { + return; + } - clientCompositionLayers.insert(clientCompositionLayers.end(), - std::make_move_iterator(results.begin()), - std::make_move_iterator(results.end())); - renderedLayers.push_back(layer); + settings->geometry.positionTransform = + transform.asMatrix4() * settings->geometry.positionTransform; + // There's no need to process blurs when we're executing region sampling, + // we're just trying to understand what we're drawing, and doing so without + // blurs is already a pretty good approximation. + if (regionSampling) { + settings->backgroundBlurRadius = 0; } + captureResults.capturedHdrLayers |= isHdrLayer(layer); + clientCompositionLayers.push_back(std::move(*settings)); + renderedLayers.push_back(layer); }); std::vector<renderengine::LayerSettings> clientRenderEngineLayers; diff --git a/services/surfaceflinger/SurfaceFlinger.h b/services/surfaceflinger/SurfaceFlinger.h index 4d4d263dbc..bbe15890bd 100644 --- a/services/surfaceflinger/SurfaceFlinger.h +++ b/services/surfaceflinger/SurfaceFlinger.h @@ -432,11 +432,6 @@ private: FINISHED, }; - struct HotplugEvent { - hal::HWDisplayId hwcDisplayId; - hal::Connection connection = hal::Connection::INVALID; - }; - template <typename F, std::enable_if_t<!std::is_member_function_pointer_v<F>>* = nullptr> static Dumper dumper(F&& dump) { using namespace std::placeholders; @@ -611,6 +606,9 @@ private: // ICompositor overrides: + // Configures physical displays, processing hotplug and/or mode setting via the Composer HAL. + void configure() override; + // Commits transactions for layers and displays. Returns whether any state has been invalidated, // i.e. whether a frame should be composited for each display. bool commit(TimePoint frameTime, VsyncId, TimePoint expectedVsyncTime) override; @@ -622,9 +620,7 @@ private: // Samples the composited frame via RegionSamplingThread. void sample() override; - /* - * ISchedulerCallback - */ + // ISchedulerCallback overrides: // Toggles hardware VSYNC by calling into HWC. void setVsyncEnabled(bool) override; @@ -634,6 +630,7 @@ private: void kernelTimerChanged(bool expired) override; // Called when the frame rate override list changed to trigger an event. void triggerOnFrameRateOverridesChanged() override; + // Toggles the kernel idle timer on or off depending the policy decisions around refresh rates. void toggleKernelIdleTimer() REQUIRES(mStateLock); // Get the controller and timeout that will help decide how the kernel idle timer will be @@ -911,6 +908,13 @@ private: std::pair<DisplayModes, DisplayModePtr> loadDisplayModes(PhysicalDisplayId) const REQUIRES(mStateLock); + // TODO(b/241285876): Move to DisplayConfigurator. + // + // Returns whether displays have been added/changed/removed, i.e. whether ICompositor should + // commit display transactions. + bool configureLocked() REQUIRES(mStateLock) REQUIRES(kMainThreadContext) + EXCLUDES(mHotplugMutex); + sp<DisplayDevice> setupNewDisplayDeviceInternal( const wp<IBinder>& displayToken, std::shared_ptr<compositionengine::Display> compositionDisplay, @@ -922,7 +926,6 @@ private: void processDisplayChanged(const wp<IBinder>& displayToken, const DisplayDeviceState& currentState, const DisplayDeviceState& drawingState) REQUIRES(mStateLock); - void processDisplayHotplugEventsLocked() REQUIRES(mStateLock); void dispatchDisplayHotplugEvent(PhysicalDisplayId displayId, bool connected); @@ -1145,7 +1148,13 @@ private: BootStage mBootStage = BootStage::BOOTLOADER; - std::vector<HotplugEvent> mPendingHotplugEvents GUARDED_BY(mStateLock); + struct HotplugEvent { + hal::HWDisplayId hwcDisplayId; + hal::Connection connection = hal::Connection::INVALID; + }; + + std::mutex mHotplugMutex; + std::vector<HotplugEvent> mPendingHotplugEvents GUARDED_BY(mHotplugMutex); // Displays are composited in `mDisplays` order. Internal displays are inserted at boot and // never removed, so take precedence over external and virtual displays. diff --git a/services/surfaceflinger/fuzzer/surfaceflinger_fuzzers_utils.h b/services/surfaceflinger/fuzzer/surfaceflinger_fuzzers_utils.h index 4c649f30ac..3aa3633bd9 100644 --- a/services/surfaceflinger/fuzzer/surfaceflinger_fuzzers_utils.h +++ b/services/surfaceflinger/fuzzer/surfaceflinger_fuzzers_utils.h @@ -278,6 +278,7 @@ public: private: // ICompositor overrides: + void configure() override {} bool commit(TimePoint, VsyncId, TimePoint) override { return false; } void composite(TimePoint, VsyncId) override {} void sample() override {} diff --git a/services/surfaceflinger/tests/unittests/DisplayDevice_InitiateModeChange.cpp b/services/surfaceflinger/tests/unittests/DisplayDevice_InitiateModeChange.cpp index 93af225b3e..982b9ff9ea 100644 --- a/services/surfaceflinger/tests/unittests/DisplayDevice_InitiateModeChange.cpp +++ b/services/surfaceflinger/tests/unittests/DisplayDevice_InitiateModeChange.cpp @@ -42,6 +42,7 @@ public: PrimaryDisplayVariant::setupHwcGetActiveConfigCallExpectations(this); mFlinger.onComposerHalHotplug(PrimaryDisplayVariant::HWC_DISPLAY_ID, Connection::CONNECTED); + mFlinger.configureAndCommit(); mDisplay = PrimaryDisplayVariant::makeFakeExistingDisplayInjector(this) .setDisplayModes(makeModes(kMode60, kMode90, kMode120), kModeId60) diff --git a/services/surfaceflinger/tests/unittests/MessageQueueTest.cpp b/services/surfaceflinger/tests/unittests/MessageQueueTest.cpp index f20b9f9c8d..5e1042e91f 100644 --- a/services/surfaceflinger/tests/unittests/MessageQueueTest.cpp +++ b/services/surfaceflinger/tests/unittests/MessageQueueTest.cpp @@ -32,6 +32,7 @@ using namespace testing; using CallbackToken = scheduler::VSyncDispatch::CallbackToken; struct NoOpCompositor final : ICompositor { + void configure() override {} bool commit(TimePoint, VsyncId, TimePoint) override { return false; } void composite(TimePoint, VsyncId) override {} void sample() override {} diff --git a/services/surfaceflinger/tests/unittests/SurfaceFlinger_DisplayModeSwitching.cpp b/services/surfaceflinger/tests/unittests/SurfaceFlinger_DisplayModeSwitching.cpp index b607b881d2..57937dcaea 100644 --- a/services/surfaceflinger/tests/unittests/SurfaceFlinger_DisplayModeSwitching.cpp +++ b/services/surfaceflinger/tests/unittests/SurfaceFlinger_DisplayModeSwitching.cpp @@ -40,18 +40,17 @@ public: PrimaryDisplayVariant::setupNativeWindowSurfaceCreationCallExpectations(this); PrimaryDisplayVariant::setupHwcGetActiveConfigCallExpectations(this); - mFlinger.onComposerHalHotplug(PrimaryDisplayVariant::HWC_DISPLAY_ID, Connection::CONNECTED); + DisplayModes modes = makeModes(kMode60, kMode90, kMode120, kMode90_4K); + auto configs = std::make_shared<scheduler::RefreshRateConfigs>(modes, kModeId60); - { - DisplayModes modes = makeModes(kMode60, kMode90, kMode120, kMode90_4K); - auto configs = std::make_shared<scheduler::RefreshRateConfigs>(modes, kModeId60); + setupScheduler(configs); - mDisplay = PrimaryDisplayVariant::makeFakeExistingDisplayInjector(this) - .setDisplayModes(std::move(modes), kModeId60, std::move(configs)) - .inject(); - } + mFlinger.onComposerHalHotplug(PrimaryDisplayVariant::HWC_DISPLAY_ID, Connection::CONNECTED); + mFlinger.configureAndCommit(); - setupScheduler(mDisplay->holdRefreshRateConfigs()); + mDisplay = PrimaryDisplayVariant::makeFakeExistingDisplayInjector(this) + .setDisplayModes(std::move(modes), kModeId60, std::move(configs)) + .inject(); // isVsyncPeriodSwitchSupported should return true, otherwise the SF's HWC proxy // will call setActiveConfig instead of setActiveConfigWithConstraints. diff --git a/services/surfaceflinger/tests/unittests/SurfaceFlinger_DisplayTransactionCommitTest.cpp b/services/surfaceflinger/tests/unittests/SurfaceFlinger_DisplayTransactionCommitTest.cpp index bc9e8016bf..71f1a2bb35 100644 --- a/services/surfaceflinger/tests/unittests/SurfaceFlinger_DisplayTransactionCommitTest.cpp +++ b/services/surfaceflinger/tests/unittests/SurfaceFlinger_DisplayTransactionCommitTest.cpp @@ -175,7 +175,7 @@ void DisplayTransactionCommitTest::processesHotplugConnectCommon() { // -------------------------------------------------------------------- // Invocation - mFlinger.commitTransactionsLocked(eDisplayTransactionNeeded); + mFlinger.configureAndCommit(); // -------------------------------------------------------------------- // Postconditions @@ -204,7 +204,7 @@ void DisplayTransactionCommitTest::ignoresHotplugConnectCommon() { // -------------------------------------------------------------------- // Invocation - mFlinger.commitTransactionsLocked(eDisplayTransactionNeeded); + mFlinger.configureAndCommit(); // -------------------------------------------------------------------- // Postconditions @@ -239,7 +239,7 @@ void DisplayTransactionCommitTest::processesHotplugDisconnectCommon() { // -------------------------------------------------------------------- // Invocation - mFlinger.commitTransactionsLocked(eDisplayTransactionNeeded); + mFlinger.configureAndCommit(); // -------------------------------------------------------------------- // Postconditions @@ -321,7 +321,7 @@ TEST_F(DisplayTransactionCommitTest, processesHotplugConnectThenDisconnectPrimar // -------------------------------------------------------------------- // Invocation - mFlinger.commitTransactionsLocked(eDisplayTransactionNeeded); + mFlinger.configureAndCommit(); // -------------------------------------------------------------------- // Postconditions @@ -366,7 +366,7 @@ TEST_F(DisplayTransactionCommitTest, processesHotplugDisconnectThenConnectPrimar // -------------------------------------------------------------------- // Invocation - mFlinger.commitTransactionsLocked(eDisplayTransactionNeeded); + mFlinger.configureAndCommit(); // -------------------------------------------------------------------- // Postconditions diff --git a/services/surfaceflinger/tests/unittests/SurfaceFlinger_HotplugTest.cpp b/services/surfaceflinger/tests/unittests/SurfaceFlinger_HotplugTest.cpp index 1da3bb6c04..e5cf4d7097 100644 --- a/services/surfaceflinger/tests/unittests/SurfaceFlinger_HotplugTest.cpp +++ b/services/surfaceflinger/tests/unittests/SurfaceFlinger_HotplugTest.cpp @@ -23,37 +23,15 @@ namespace android { class HotplugTest : public DisplayTransactionTest {}; -TEST_F(HotplugTest, enqueuesEventsForDisplayTransaction) { - constexpr HWDisplayId hwcDisplayId1 = 456; - constexpr HWDisplayId hwcDisplayId2 = 654; - - // -------------------------------------------------------------------- - // Preconditions - - // Set the main thread id so that the current thread does not appear to be - // the main thread. - mFlinger.mutableMainThreadId() = std::thread::id(); - - // -------------------------------------------------------------------- - // Call Expectations - - // We expect a scheduled commit for the display transaction. - EXPECT_CALL(*mFlinger.scheduler(), scheduleFrame()).Times(1); +TEST_F(HotplugTest, schedulesConfigureToProcessHotplugEvents) { + EXPECT_CALL(*mFlinger.scheduler(), scheduleConfigure()).Times(2); - // -------------------------------------------------------------------- - // Invocation - - // Simulate two hotplug events (a connect and a disconnect) + constexpr HWDisplayId hwcDisplayId1 = 456; mFlinger.onComposerHalHotplug(hwcDisplayId1, Connection::CONNECTED); - mFlinger.onComposerHalHotplug(hwcDisplayId2, Connection::DISCONNECTED); - // -------------------------------------------------------------------- - // Postconditions - - // The display transaction needed flag should be set. - EXPECT_TRUE(hasTransactionFlagSet(eDisplayTransactionNeeded)); + constexpr HWDisplayId hwcDisplayId2 = 654; + mFlinger.onComposerHalHotplug(hwcDisplayId2, Connection::DISCONNECTED); - // All events should be in the pending event queue. const auto& pendingEvents = mFlinger.mutablePendingHotplugEvents(); ASSERT_EQ(2u, pendingEvents.size()); EXPECT_EQ(hwcDisplayId1, pendingEvents[0].hwcDisplayId); @@ -62,48 +40,16 @@ TEST_F(HotplugTest, enqueuesEventsForDisplayTransaction) { EXPECT_EQ(Connection::DISCONNECTED, pendingEvents[1].connection); } -TEST_F(HotplugTest, processesEnqueuedEventsIfCalledOnMainThread) { - constexpr HWDisplayId displayId1 = 456; - - // -------------------------------------------------------------------- - // Note: - // -------------------------------------------------------------------- - // This test case is a bit tricky. We want to verify that - // onComposerHalHotplug() calls processDisplayHotplugEventsLocked(), but we - // don't really want to provide coverage for everything the later function - // does as there are specific tests for it. - // -------------------------------------------------------------------- - - // -------------------------------------------------------------------- - // Preconditions - - // Set the main thread id so that the current thread does appear to be the - // main thread. - mFlinger.mutableMainThreadId() = std::this_thread::get_id(); - - // -------------------------------------------------------------------- - // Call Expectations - - // We expect a scheduled commit for the display transaction. +TEST_F(HotplugTest, schedulesFrameToCommitDisplayTransaction) { EXPECT_CALL(*mFlinger.scheduler(), scheduleFrame()).Times(1); - // -------------------------------------------------------------------- - // Invocation - - // Simulate a disconnect on a display id that is not connected. This should - // be enqueued by onComposerHalHotplug(), and dequeued by - // processDisplayHotplugEventsLocked(), but then ignored as invalid. + constexpr HWDisplayId displayId1 = 456; mFlinger.onComposerHalHotplug(displayId1, Connection::DISCONNECTED); + mFlinger.configure(); - // -------------------------------------------------------------------- - // Postconditions - - // The display transaction needed flag should be set. - EXPECT_TRUE(hasTransactionFlagSet(eDisplayTransactionNeeded)); - - // There should be no event queued on return, as it should have been - // processed. + // The configure stage should consume the hotplug queue and produce a display transaction. EXPECT_TRUE(mFlinger.mutablePendingHotplugEvents().empty()); + EXPECT_TRUE(hasTransactionFlagSet(eDisplayTransactionNeeded)); } TEST_F(HotplugTest, rejectsHotplugIfFailedToLoadDisplayModes) { @@ -125,14 +71,14 @@ TEST_F(HotplugTest, rejectsHotplugIfFailedToLoadDisplayModes) { .WillOnce(Return(Error::NONE)); ExternalDisplay::injectPendingHotplugEvent(this, Connection::CONNECTED); - mFlinger.processDisplayHotplugEvents(); + mFlinger.configure(); // The hotplug should be rejected, so no HWComposer::DisplayData should be created. EXPECT_FALSE(hasPhysicalHwcDisplay(ExternalDisplay::HWC_DISPLAY_ID)); // Disconnecting a display that does not exist should be a no-op. ExternalDisplay::injectPendingHotplugEvent(this, Connection::DISCONNECTED); - mFlinger.processDisplayHotplugEvents(); + mFlinger.configure(); EXPECT_FALSE(hasPhysicalHwcDisplay(ExternalDisplay::HWC_DISPLAY_ID)); } diff --git a/services/surfaceflinger/tests/unittests/TestableScheduler.h b/services/surfaceflinger/tests/unittests/TestableScheduler.h index 66cdfd7b29..93e305960c 100644 --- a/services/surfaceflinger/tests/unittests/TestableScheduler.h +++ b/services/surfaceflinger/tests/unittests/TestableScheduler.h @@ -50,6 +50,7 @@ public: }); } + MOCK_METHOD(void, scheduleConfigure, (), (override)); MOCK_METHOD(void, scheduleFrame, (), (override)); MOCK_METHOD(void, postMessage, (sp<MessageHandler>&&), (override)); @@ -109,6 +110,7 @@ public: private: // ICompositor overrides: + void configure() override {} bool commit(TimePoint, VsyncId, TimePoint) override { return false; } void composite(TimePoint, VsyncId) override {} void sample() override {} diff --git a/services/surfaceflinger/tests/unittests/TestableSurfaceFlinger.h b/services/surfaceflinger/tests/unittests/TestableSurfaceFlinger.h index 7dd9935683..42585b579b 100644 --- a/services/surfaceflinger/tests/unittests/TestableSurfaceFlinger.h +++ b/services/surfaceflinger/tests/unittests/TestableSurfaceFlinger.h @@ -317,8 +317,15 @@ public: * Forwarding for functions being tested */ - TimePoint commit(TimePoint frameTime, VsyncId vsyncId, TimePoint expectedVSyncTime) { - mFlinger->commit(frameTime, vsyncId, expectedVSyncTime); + void configure() { mFlinger->configure(); } + + void configureAndCommit() { + configure(); + commitTransactionsLocked(eDisplayTransactionNeeded); + } + + TimePoint commit(TimePoint frameTime, VsyncId vsyncId, TimePoint expectedVsyncTime) { + mFlinger->commit(frameTime, vsyncId, expectedVsyncTime); return frameTime; } @@ -362,19 +369,15 @@ public: dispSurface, producer); } - auto commitTransactionsLocked(uint32_t transactionFlags) { + void commitTransactionsLocked(uint32_t transactionFlags) { Mutex::Autolock lock(mFlinger->mStateLock); - return mFlinger->commitTransactionsLocked(transactionFlags); + mFlinger->commitTransactionsLocked(transactionFlags); } void onComposerHalHotplug(hal::HWDisplayId hwcDisplayId, hal::Connection connection) { mFlinger->onComposerHalHotplug(hwcDisplayId, connection); } - void processDisplayHotplugEvents() { - FTL_FAKE_GUARD(mFlinger->mStateLock, mFlinger->processDisplayHotplugEventsLocked()); - } - auto setDisplayStateLocked(const DisplayState& s) { Mutex::Autolock lock(mFlinger->mStateLock); return mFlinger->setDisplayStateLocked(s); |