diff options
98 files changed, 1007 insertions, 607 deletions
diff --git a/cmds/dumpstate/DumpstateService.cpp b/cmds/dumpstate/DumpstateService.cpp index ba0a38aad9..a8d12a1a91 100644 --- a/cmds/dumpstate/DumpstateService.cpp +++ b/cmds/dumpstate/DumpstateService.cpp @@ -39,6 +39,7 @@ struct DumpstateInfo { std::string calling_package; int32_t user_id = -1; bool keep_bugreport_on_retrieval = false; + bool skip_user_consent = false; }; static binder::Status exception(uint32_t code, const std::string& msg, @@ -62,7 +63,8 @@ static binder::Status exception(uint32_t code, const std::string& msg, [[noreturn]] static void* dumpstate_thread_retrieve(void* data) { std::unique_ptr<DumpstateInfo> ds_info(static_cast<DumpstateInfo*>(data)); - ds_info->ds->Retrieve(ds_info->calling_uid, ds_info->calling_package, ds_info->keep_bugreport_on_retrieval); + ds_info->ds->Retrieve(ds_info->calling_uid, ds_info->calling_package, + ds_info->keep_bugreport_on_retrieval, ds_info->skip_user_consent); MYLOGD("Finished retrieving a bugreport. Exiting.\n"); exit(0); } @@ -116,7 +118,8 @@ binder::Status DumpstateService::startBugreport(int32_t calling_uid, int bugreport_mode, int bugreport_flags, const sp<IDumpstateListener>& listener, - bool is_screenshot_requested) { + bool is_screenshot_requested, + bool skip_user_consent) { MYLOGI("startBugreport() with mode: %d\n", bugreport_mode); // Ensure there is only one bugreport in progress at a time. @@ -151,7 +154,7 @@ binder::Status DumpstateService::startBugreport(int32_t calling_uid, std::unique_ptr<Dumpstate::DumpOptions> options = std::make_unique<Dumpstate::DumpOptions>(); options->Initialize(static_cast<Dumpstate::BugreportMode>(bugreport_mode), bugreport_flags, - bugreport_fd, screenshot_fd, is_screenshot_requested); + bugreport_fd, screenshot_fd, is_screenshot_requested, skip_user_consent); if (bugreport_fd.get() == -1 || (options->do_screenshot && screenshot_fd.get() == -1)) { MYLOGE("Invalid filedescriptor"); @@ -207,6 +210,7 @@ binder::Status DumpstateService::retrieveBugreport( android::base::unique_fd bugreport_fd, const std::string& bugreport_file, const bool keep_bugreport_on_retrieval, + const bool skip_user_consent, const sp<IDumpstateListener>& listener) { ds_ = &(Dumpstate::GetInstance()); @@ -216,6 +220,7 @@ binder::Status DumpstateService::retrieveBugreport( ds_info->calling_package = calling_package; ds_info->user_id = user_id; ds_info->keep_bugreport_on_retrieval = keep_bugreport_on_retrieval; + ds_info->skip_user_consent = skip_user_consent; ds_->listener_ = listener; std::unique_ptr<Dumpstate::DumpOptions> options = std::make_unique<Dumpstate::DumpOptions>(); // Use a /dev/null FD when initializing options since none is provided. @@ -223,7 +228,7 @@ binder::Status DumpstateService::retrieveBugreport( TEMP_FAILURE_RETRY(open("/dev/null", O_WRONLY | O_CLOEXEC))); options->Initialize(Dumpstate::BugreportMode::BUGREPORT_DEFAULT, - 0, bugreport_fd, devnull_fd, false); + 0, bugreport_fd, devnull_fd, false, skip_user_consent); if (bugreport_fd.get() == -1) { MYLOGE("Invalid filedescriptor"); diff --git a/cmds/dumpstate/DumpstateService.h b/cmds/dumpstate/DumpstateService.h index 7b76c36380..c99f70eb1b 100644 --- a/cmds/dumpstate/DumpstateService.h +++ b/cmds/dumpstate/DumpstateService.h @@ -44,7 +44,7 @@ class DumpstateService : public BinderService<DumpstateService>, public BnDumpst android::base::unique_fd bugreport_fd, android::base::unique_fd screenshot_fd, int bugreport_mode, int bugreport_flags, const sp<IDumpstateListener>& listener, - bool is_screenshot_requested) override; + bool is_screenshot_requested, bool skip_user_consent) override; binder::Status retrieveBugreport(int32_t calling_uid, const std::string& calling_package, @@ -52,6 +52,7 @@ class DumpstateService : public BinderService<DumpstateService>, public BnDumpst android::base::unique_fd bugreport_fd, const std::string& bugreport_file, const bool keep_bugreport_on_retrieval, + const bool skip_user_consent, const sp<IDumpstateListener>& listener) override; diff --git a/cmds/dumpstate/DumpstateUtil.h b/cmds/dumpstate/DumpstateUtil.h index 9e955e3c04..ae7152a26a 100644 --- a/cmds/dumpstate/DumpstateUtil.h +++ b/cmds/dumpstate/DumpstateUtil.h @@ -18,6 +18,7 @@ #include <cstdint> #include <string> +#include <vector> /* * Converts seconds to milliseconds. diff --git a/cmds/dumpstate/binder/android/os/IDumpstate.aidl b/cmds/dumpstate/binder/android/os/IDumpstate.aidl index 97c470e08c..3b8fde9f51 100644 --- a/cmds/dumpstate/binder/android/os/IDumpstate.aidl +++ b/cmds/dumpstate/binder/android/os/IDumpstate.aidl @@ -96,7 +96,8 @@ interface IDumpstate { void startBugreport(int callingUid, @utf8InCpp String callingPackage, FileDescriptor bugreportFd, FileDescriptor screenshotFd, int bugreportMode, int bugreportFlags, - IDumpstateListener listener, boolean isScreenshotRequested); + IDumpstateListener listener, boolean isScreenshotRequested, + boolean skipUserConsent); /** * Cancels the bugreport currently in progress. @@ -130,5 +131,6 @@ interface IDumpstate { FileDescriptor bugreportFd, @utf8InCpp String bugreportFile, boolean keepBugreportOnRetrieval, + boolean skipUserConsent, IDumpstateListener listener); } diff --git a/cmds/dumpstate/dumpstate.cpp b/cmds/dumpstate/dumpstate.cpp index 6b9a0a0e7e..d5125f01d6 100644 --- a/cmds/dumpstate/dumpstate.cpp +++ b/cmds/dumpstate/dumpstate.cpp @@ -2902,9 +2902,11 @@ void Dumpstate::DumpOptions::Initialize(BugreportMode bugreport_mode, int bugreport_flags, const android::base::unique_fd& bugreport_fd_in, const android::base::unique_fd& screenshot_fd_in, - bool is_screenshot_requested) { + bool is_screenshot_requested, + bool skip_user_consent) { this->use_predumped_ui_data = bugreport_flags & BugreportFlag::BUGREPORT_USE_PREDUMPED_UI_DATA; this->is_consent_deferred = bugreport_flags & BugreportFlag::BUGREPORT_FLAG_DEFER_CONSENT; + this->skip_user_consent = skip_user_consent; // Duplicate the fds because the passed in fds don't outlive the binder transaction. bugreport_fd.reset(fcntl(bugreport_fd_in.get(), F_DUPFD_CLOEXEC, 0)); screenshot_fd.reset(fcntl(screenshot_fd_in.get(), F_DUPFD_CLOEXEC, 0)); @@ -2992,46 +2994,52 @@ Dumpstate::RunStatus Dumpstate::Run(int32_t calling_uid, const std::string& call } Dumpstate::RunStatus Dumpstate::Retrieve(int32_t calling_uid, const std::string& calling_package, - const bool keep_bugreport_on_retrieval) { + const bool keep_bugreport_on_retrieval, + const bool skip_user_consent) { Dumpstate::RunStatus status = RetrieveInternal(calling_uid, calling_package, - keep_bugreport_on_retrieval); + keep_bugreport_on_retrieval, + skip_user_consent); HandleRunStatus(status); return status; } Dumpstate::RunStatus Dumpstate::RetrieveInternal(int32_t calling_uid, const std::string& calling_package, - const bool keep_bugreport_on_retrieval) { - consent_callback_ = new ConsentCallback(); - const String16 incidentcompanion("incidentcompanion"); - sp<android::IBinder> ics( - defaultServiceManager()->checkService(incidentcompanion)); - android::String16 package(calling_package.c_str()); - if (ics != nullptr) { - MYLOGD("Checking user consent via incidentcompanion service\n"); - android::interface_cast<android::os::IIncidentCompanion>(ics)->authorizeReport( - calling_uid, package, String16(), String16(), - 0x1 /* FLAG_CONFIRMATION_DIALOG */, consent_callback_.get()); - } else { - MYLOGD( - "Unable to check user consent; incidentcompanion service unavailable\n"); - return RunStatus::USER_CONSENT_TIMED_OUT; - } - UserConsentResult consent_result = consent_callback_->getResult(); - int timeout_ms = 30 * 1000; - while (consent_result == UserConsentResult::UNAVAILABLE && - consent_callback_->getElapsedTimeMs() < timeout_ms) { - sleep(1); - consent_result = consent_callback_->getResult(); - } - if (consent_result == UserConsentResult::DENIED) { - return RunStatus::USER_CONSENT_DENIED; - } - if (consent_result == UserConsentResult::UNAVAILABLE) { - MYLOGD("Canceling user consent request via incidentcompanion service\n"); - android::interface_cast<android::os::IIncidentCompanion>(ics)->cancelAuthorization( - consent_callback_.get()); - return RunStatus::USER_CONSENT_TIMED_OUT; + const bool keep_bugreport_on_retrieval, + const bool skip_user_consent) { + if (!android::app::admin::flags::onboarding_consentless_bugreports() || !skip_user_consent) { + consent_callback_ = new ConsentCallback(); + const String16 incidentcompanion("incidentcompanion"); + sp<android::IBinder> ics( + defaultServiceManager()->checkService(incidentcompanion)); + android::String16 package(calling_package.c_str()); + if (ics != nullptr) { + MYLOGD("Checking user consent via incidentcompanion service\n"); + + android::interface_cast<android::os::IIncidentCompanion>(ics)->authorizeReport( + calling_uid, package, String16(), String16(), + 0x1 /* FLAG_CONFIRMATION_DIALOG */, consent_callback_.get()); + } else { + MYLOGD( + "Unable to check user consent; incidentcompanion service unavailable\n"); + return RunStatus::USER_CONSENT_TIMED_OUT; + } + UserConsentResult consent_result = consent_callback_->getResult(); + int timeout_ms = 30 * 1000; + while (consent_result == UserConsentResult::UNAVAILABLE && + consent_callback_->getElapsedTimeMs() < timeout_ms) { + sleep(1); + consent_result = consent_callback_->getResult(); + } + if (consent_result == UserConsentResult::DENIED) { + return RunStatus::USER_CONSENT_DENIED; + } + if (consent_result == UserConsentResult::UNAVAILABLE) { + MYLOGD("Canceling user consent request via incidentcompanion service\n"); + android::interface_cast<android::os::IIncidentCompanion>(ics)->cancelAuthorization( + consent_callback_.get()); + return RunStatus::USER_CONSENT_TIMED_OUT; + } } bool copy_succeeded = @@ -3510,7 +3518,9 @@ void Dumpstate::onUiIntensiveBugreportDumpsFinished(int32_t calling_uid) { void Dumpstate::MaybeCheckUserConsent(int32_t calling_uid, const std::string& calling_package) { if (multiuser_get_app_id(calling_uid) == AID_SHELL || - !CalledByApi() || options_->is_consent_deferred) { + !CalledByApi() || options_->is_consent_deferred || + (android::app::admin::flags::onboarding_consentless_bugreports() && + options_->skip_user_consent)) { // No need to get consent for shell triggered dumpstates, or not // through bugreporting API (i.e. no fd to copy back), or when consent // is deferred. @@ -3596,7 +3606,8 @@ Dumpstate::RunStatus Dumpstate::CopyBugreportIfUserConsented(int32_t calling_uid // If the caller has asked to copy the bugreport over to their directory, we need explicit // user consent (unless the caller is Shell). UserConsentResult consent_result; - if (multiuser_get_app_id(calling_uid) == AID_SHELL) { + if (multiuser_get_app_id(calling_uid) == AID_SHELL || (options_->skip_user_consent + && android::app::admin::flags::onboarding_consentless_bugreports())) { consent_result = UserConsentResult::APPROVED; } else { consent_result = consent_callback_->getResult(); diff --git a/cmds/dumpstate/dumpstate.h b/cmds/dumpstate/dumpstate.h index 46d949e303..fcb8cf3c07 100644 --- a/cmds/dumpstate/dumpstate.h +++ b/cmds/dumpstate/dumpstate.h @@ -364,7 +364,7 @@ class Dumpstate { * Initialize() dumpstate before calling this method. */ RunStatus Retrieve(int32_t calling_uid, const std::string& calling_package, - const bool keep_bugreport_on_retrieval); + const bool keep_bugreport_on_retrieval, const bool skip_user_consent); @@ -412,6 +412,7 @@ class Dumpstate { bool do_screenshot = false; bool is_screenshot_copied = false; bool is_consent_deferred = false; + bool skip_user_consent = false; bool is_remote_mode = false; bool show_header_only = false; bool telephony_only = false; @@ -448,7 +449,8 @@ class Dumpstate { void Initialize(BugreportMode bugreport_mode, int bugreport_flags, const android::base::unique_fd& bugreport_fd, const android::base::unique_fd& screenshot_fd, - bool is_screenshot_requested); + bool is_screenshot_requested, + bool skip_user_consent); /* Returns true if the options set so far are consistent. */ bool ValidateOptions() const; @@ -564,7 +566,8 @@ class Dumpstate { private: RunStatus RunInternal(int32_t calling_uid, const std::string& calling_package); RunStatus RetrieveInternal(int32_t calling_uid, const std::string& calling_package, - const bool keep_bugreport_on_retrieval); + const bool keep_bugreport_on_retrieval, + const bool skip_user_consent); RunStatus DumpstateDefaultAfterCritical(); RunStatus dumpstate(); diff --git a/cmds/dumpstate/tests/dumpstate_smoke_test.cpp b/cmds/dumpstate/tests/dumpstate_smoke_test.cpp index ccf64fe54e..a29923a4c1 100644 --- a/cmds/dumpstate/tests/dumpstate_smoke_test.cpp +++ b/cmds/dumpstate/tests/dumpstate_smoke_test.cpp @@ -507,7 +507,7 @@ TEST_F(DumpstateBinderTest, Baseline) { ds_binder->startBugreport(123, "com.example.package", std::move(bugreport_fd), std::move(screenshot_fd), Dumpstate::BugreportMode::BUGREPORT_INTERACTIVE, flags, listener, - true); + true, false); // startBugreport is an async call. Verify binder call succeeded first, then wait till listener // gets expected callbacks. EXPECT_TRUE(status.isOk()); @@ -545,7 +545,7 @@ TEST_F(DumpstateBinderTest, ServiceDies_OnInvalidInput) { android::binder::Status status = ds_binder->startBugreport(123, "com.example.package", std::move(bugreport_fd), std::move(screenshot_fd), 2000, // invalid bugreport mode - flags, listener, false); + flags, listener, false, false); EXPECT_EQ(listener->getErrorCode(), IDumpstateListener::BUGREPORT_ERROR_INVALID_INPUT); // The service should have died, freeing itself up for a new invocation. @@ -579,7 +579,7 @@ TEST_F(DumpstateBinderTest, SimultaneousBugreportsNotAllowed) { ds_binder->startBugreport(123, "com.example.package", std::move(bugreport_fd), std::move(screenshot_fd), Dumpstate::BugreportMode::BUGREPORT_INTERACTIVE, flags, listener1, - true); + true, false); EXPECT_TRUE(status.isOk()); // try to make another call to startBugreport. This should fail. @@ -587,7 +587,7 @@ TEST_F(DumpstateBinderTest, SimultaneousBugreportsNotAllowed) { status = ds_binder->startBugreport(123, "com.example.package", std::move(bugreport_fd2), std::move(screenshot_fd2), Dumpstate::BugreportMode::BUGREPORT_INTERACTIVE, flags, - listener2, true); + listener2, true, false); EXPECT_FALSE(status.isOk()); WaitTillExecutionComplete(listener2.get()); EXPECT_EQ(listener2->getErrorCode(), diff --git a/cmds/dumpstate/tests/dumpstate_test.cpp b/cmds/dumpstate/tests/dumpstate_test.cpp index 2afabed813..18c2f94432 100644 --- a/cmds/dumpstate/tests/dumpstate_test.cpp +++ b/cmds/dumpstate/tests/dumpstate_test.cpp @@ -239,7 +239,7 @@ TEST_F(DumpOptionsTest, InitializeAdbShellBugreport) { } TEST_F(DumpOptionsTest, InitializeFullBugReport) { - options_.Initialize(Dumpstate::BugreportMode::BUGREPORT_FULL, 0, fd, fd, true); + options_.Initialize(Dumpstate::BugreportMode::BUGREPORT_FULL, 0, fd, fd, true, false); EXPECT_TRUE(options_.do_screenshot); // Other options retain default values @@ -253,7 +253,7 @@ TEST_F(DumpOptionsTest, InitializeFullBugReport) { } TEST_F(DumpOptionsTest, InitializeInteractiveBugReport) { - options_.Initialize(Dumpstate::BugreportMode::BUGREPORT_INTERACTIVE, 0, fd, fd, true); + options_.Initialize(Dumpstate::BugreportMode::BUGREPORT_INTERACTIVE, 0, fd, fd, true, false); EXPECT_TRUE(options_.do_progress_updates); EXPECT_TRUE(options_.do_screenshot); @@ -267,7 +267,7 @@ TEST_F(DumpOptionsTest, InitializeInteractiveBugReport) { } TEST_F(DumpOptionsTest, InitializeRemoteBugReport) { - options_.Initialize(Dumpstate::BugreportMode::BUGREPORT_REMOTE, 0, fd, fd, false); + options_.Initialize(Dumpstate::BugreportMode::BUGREPORT_REMOTE, 0, fd, fd, false, false); EXPECT_TRUE(options_.is_remote_mode); EXPECT_FALSE(options_.do_vibrate); EXPECT_FALSE(options_.do_screenshot); @@ -281,7 +281,7 @@ TEST_F(DumpOptionsTest, InitializeRemoteBugReport) { } TEST_F(DumpOptionsTest, InitializeWearBugReport) { - options_.Initialize(Dumpstate::BugreportMode::BUGREPORT_WEAR, 0, fd, fd, true); + options_.Initialize(Dumpstate::BugreportMode::BUGREPORT_WEAR, 0, fd, fd, true, false); EXPECT_TRUE(options_.do_screenshot); EXPECT_TRUE(options_.do_progress_updates); @@ -296,7 +296,7 @@ TEST_F(DumpOptionsTest, InitializeWearBugReport) { } TEST_F(DumpOptionsTest, InitializeTelephonyBugReport) { - options_.Initialize(Dumpstate::BugreportMode::BUGREPORT_TELEPHONY, 0, fd, fd, false); + options_.Initialize(Dumpstate::BugreportMode::BUGREPORT_TELEPHONY, 0, fd, fd, false, false); EXPECT_FALSE(options_.do_screenshot); EXPECT_TRUE(options_.telephony_only); EXPECT_TRUE(options_.do_progress_updates); @@ -311,7 +311,7 @@ TEST_F(DumpOptionsTest, InitializeTelephonyBugReport) { } TEST_F(DumpOptionsTest, InitializeWifiBugReport) { - options_.Initialize(Dumpstate::BugreportMode::BUGREPORT_WIFI, 0, fd, fd, false); + options_.Initialize(Dumpstate::BugreportMode::BUGREPORT_WIFI, 0, fd, fd, false, false); EXPECT_FALSE(options_.do_screenshot); EXPECT_TRUE(options_.wifi_only); @@ -491,12 +491,12 @@ TEST_F(DumpOptionsTest, InitializeBugreportFlags) { int flags = Dumpstate::BugreportFlag::BUGREPORT_USE_PREDUMPED_UI_DATA | Dumpstate::BugreportFlag::BUGREPORT_FLAG_DEFER_CONSENT; options_.Initialize( - Dumpstate::BugreportMode::BUGREPORT_FULL, flags, fd, fd, true); + Dumpstate::BugreportMode::BUGREPORT_FULL, flags, fd, fd, true, false); EXPECT_TRUE(options_.is_consent_deferred); EXPECT_TRUE(options_.use_predumped_ui_data); options_.Initialize( - Dumpstate::BugreportMode::BUGREPORT_FULL, 0, fd, fd, true); + Dumpstate::BugreportMode::BUGREPORT_FULL, 0, fd, fd, true, false); EXPECT_FALSE(options_.is_consent_deferred); EXPECT_FALSE(options_.use_predumped_ui_data); } diff --git a/libs/binder/BpBinder.cpp b/libs/binder/BpBinder.cpp index 54457fc568..7a855e1c5b 100644 --- a/libs/binder/BpBinder.cpp +++ b/libs/binder/BpBinder.cpp @@ -15,6 +15,7 @@ */ #define LOG_TAG "BpBinder" +#define ATRACE_TAG ATRACE_TAG_AIDL //#define LOG_NDEBUG 0 #include <binder/BpBinder.h> @@ -26,6 +27,12 @@ #include <stdio.h> +#ifndef __TRUSTY__ +#include <cutils/trace.h> +#else +#define ATRACE_INT(...) +#endif + #include "BuildFlags.h" #include "file.h" @@ -209,6 +216,7 @@ sp<BpBinder> BpBinder::create(int32_t handle) { sTrackingMap[trackedUid]++; } uint32_t numProxies = sBinderProxyCount.fetch_add(1, std::memory_order_relaxed); + ATRACE_INT("binder_proxies", numProxies); uint32_t numLastWarned = sBinderProxyCountWarned.load(std::memory_order_relaxed); uint32_t numNextWarn = numLastWarned + kBinderProxyCountWarnInterval; if (numProxies >= numNextWarn) { @@ -632,8 +640,8 @@ BpBinder::~BpBinder() { } } } - --sBinderProxyCount; - + [[maybe_unused]] uint32_t numProxies = --sBinderProxyCount; + ATRACE_INT("binder_proxies", numProxies); if (ipc) { ipc->expungeHandle(binderHandle(), this); ipc->decWeakHandle(binderHandle()); diff --git a/libs/binder/IServiceManager.cpp b/libs/binder/IServiceManager.cpp index 39573ec54d..fbcf823045 100644 --- a/libs/binder/IServiceManager.cpp +++ b/libs/binder/IServiceManager.cpp @@ -20,6 +20,7 @@ #include <inttypes.h> #include <unistd.h> +#include <condition_variable> #include <android-base/properties.h> #include <android/os/BnServiceCallback.h> @@ -642,7 +643,7 @@ public: protected: // Override realGetService for ServiceManagerShim::waitForService. - Status realGetService(const std::string& name, sp<IBinder>* _aidl_return) { + Status realGetService(const std::string& name, sp<IBinder>* _aidl_return) override { *_aidl_return = getDeviceService({"-g", name}, mOptions); return Status::ok(); } diff --git a/libs/binder/Parcel.cpp b/libs/binder/Parcel.cpp index 611169d4bf..3f70e8c104 100644 --- a/libs/binder/Parcel.cpp +++ b/libs/binder/Parcel.cpp @@ -2768,7 +2768,7 @@ void Parcel::ipcSetDataReference(const uint8_t* data, size_t dataSize, const bin } if (type == BINDER_TYPE_FD) { // FDs from the kernel are always owned - FdTag(flat->handle, 0, this); + FdTag(flat->handle, nullptr, this); } minOffset = offset + sizeof(flat_binder_object); } diff --git a/libs/binder/RecordedTransaction.cpp b/libs/binder/RecordedTransaction.cpp index de2a69f7cf..924537e63a 100644 --- a/libs/binder/RecordedTransaction.cpp +++ b/libs/binder/RecordedTransaction.cpp @@ -230,8 +230,8 @@ std::optional<RecordedTransaction> RecordedTransaction::fromFile(const unique_fd } size_t memoryMappedSize = chunkPayloadSize + mmapPayloadStartOffset; - void* mappedMemory = - mmap(NULL, memoryMappedSize, PROT_READ, MAP_SHARED, fd.get(), mmapPageAlignedStart); + void* mappedMemory = mmap(nullptr, memoryMappedSize, PROT_READ, MAP_SHARED, fd.get(), + mmapPageAlignedStart); auto mmap_guard = make_scope_guard( [mappedMemory, memoryMappedSize] { munmap(mappedMemory, memoryMappedSize); }); @@ -382,7 +382,7 @@ android::status_t RecordedTransaction::dumpToFile(const unique_fd& fd) const { return UNKNOWN_ERROR; } - if (NO_ERROR != writeChunk(fd, END_CHUNK, 0, NULL)) { + if (NO_ERROR != writeChunk(fd, END_CHUNK, 0, nullptr)) { ALOGE("Failed to write end chunk to fd %d", fd.get()); return UNKNOWN_ERROR; } diff --git a/libs/binder/Stability.cpp b/libs/binder/Stability.cpp index 665dfea456..4fb8fa08bc 100644 --- a/libs/binder/Stability.cpp +++ b/libs/binder/Stability.cpp @@ -70,7 +70,7 @@ bool Stability::requiresVintfDeclaration(const sp<IBinder>& binder) { } void Stability::tryMarkCompilationUnit(IBinder* binder) { - (void)setRepr(binder, getLocalLevel(), REPR_NONE); + std::ignore = setRepr(binder, getLocalLevel(), REPR_NONE); } // after deprecation of the VNDK, these should be aliases. At some point diff --git a/libs/binder/include/binder/Parcel.h b/libs/binder/include/binder/Parcel.h index 5e18b9197d..eb730374d9 100644 --- a/libs/binder/include/binder/Parcel.h +++ b/libs/binder/include/binder/Parcel.h @@ -1009,7 +1009,7 @@ private: typename std::enable_if_t<is_specialization_v<CT, std::vector>, bool> = true> status_t writeData(const CT& c) { using T = first_template_type_t<CT>; // The T in CT == C<T, ...> - if (c.size() > std::numeric_limits<int32_t>::max()) return BAD_VALUE; + if (c.size() > static_cast<size_t>(std::numeric_limits<int32_t>::max())) return BAD_VALUE; const auto size = static_cast<int32_t>(c.size()); writeData(size); if constexpr (is_pointer_equivalent_array_v<T>) { diff --git a/libs/binder/ndk/include_cpp/android/binder_auto_utils.h b/libs/binder/ndk/include_cpp/android/binder_auto_utils.h index 18769b1454..8c62924beb 100644 --- a/libs/binder/ndk/include_cpp/android/binder_auto_utils.h +++ b/libs/binder/ndk/include_cpp/android/binder_auto_utils.h @@ -377,7 +377,7 @@ class ScopedAIBinder_Weak namespace internal { -static void closeWithError(int fd) { +inline void closeWithError(int fd) { if (fd == -1) return; int ret = close(fd); if (ret != 0) { diff --git a/libs/binder/rust/Android.bp b/libs/binder/rust/Android.bp index ef556d7f5f..725744cd70 100644 --- a/libs/binder/rust/Android.bp +++ b/libs/binder/rust/Android.bp @@ -59,6 +59,7 @@ rust_library { ], host_supported: true, vendor_available: true, + product_available: true, target: { darwin: { enabled: false, diff --git a/libs/binder/tests/Android.bp b/libs/binder/tests/Android.bp index 6800a8d36c..bd24a20370 100644 --- a/libs/binder/tests/Android.bp +++ b/libs/binder/tests/Android.bp @@ -28,6 +28,11 @@ cc_defaults { cflags: [ "-Wall", "-Werror", + "-Wformat", + "-Wpessimizing-move", + "-Wsign-compare", + "-Wunused-result", + "-Wzero-as-null-pointer-constant", ], } diff --git a/libs/binder/tests/RpcTlsUtilsTest.cpp b/libs/binder/tests/RpcTlsUtilsTest.cpp index 530606c44a..48e3345ed1 100644 --- a/libs/binder/tests/RpcTlsUtilsTest.cpp +++ b/libs/binder/tests/RpcTlsUtilsTest.cpp @@ -52,9 +52,9 @@ TEST_P(RpcTlsUtilsKeyTest, Test) { << "\nactual: " << toDebugString(deserializedPkey.get()); } -INSTANTIATE_TEST_CASE_P(RpcTlsUtilsTest, RpcTlsUtilsKeyTest, - testing::Values(RpcKeyFormat::PEM, RpcKeyFormat::DER), - RpcTlsUtilsKeyTest::PrintParamInfo); +INSTANTIATE_TEST_SUITE_P(RpcTlsUtilsTest, RpcTlsUtilsKeyTest, + testing::Values(RpcKeyFormat::PEM, RpcKeyFormat::DER), + RpcTlsUtilsKeyTest::PrintParamInfo); class RpcTlsUtilsCertTest : public testing::TestWithParam<RpcCertificateFormat> { public: @@ -75,9 +75,9 @@ TEST_P(RpcTlsUtilsCertTest, Test) { EXPECT_EQ(0, X509_cmp(cert.get(), deserializedCert.get())); } -INSTANTIATE_TEST_CASE_P(RpcTlsUtilsTest, RpcTlsUtilsCertTest, - testing::Values(RpcCertificateFormat::PEM, RpcCertificateFormat::DER), - RpcTlsUtilsCertTest::PrintParamInfo); +INSTANTIATE_TEST_SUITE_P(RpcTlsUtilsTest, RpcTlsUtilsCertTest, + testing::Values(RpcCertificateFormat::PEM, RpcCertificateFormat::DER), + RpcTlsUtilsCertTest::PrintParamInfo); class RpcTlsUtilsKeyAndCertTest : public testing::TestWithParam<std::tuple<RpcKeyFormat, RpcCertificateFormat>> { @@ -105,10 +105,10 @@ TEST_P(RpcTlsUtilsKeyAndCertTest, TestCertFromDeserializedKey) { EXPECT_EQ(0, X509_cmp(cert.get(), deserializedCert.get())); } -INSTANTIATE_TEST_CASE_P(RpcTlsUtilsTest, RpcTlsUtilsKeyAndCertTest, - testing::Combine(testing::Values(RpcKeyFormat::PEM, RpcKeyFormat::DER), - testing::Values(RpcCertificateFormat::PEM, - RpcCertificateFormat::DER)), - RpcTlsUtilsKeyAndCertTest::PrintParamInfo); +INSTANTIATE_TEST_SUITE_P(RpcTlsUtilsTest, RpcTlsUtilsKeyAndCertTest, + testing::Combine(testing::Values(RpcKeyFormat::PEM, RpcKeyFormat::DER), + testing::Values(RpcCertificateFormat::PEM, + RpcCertificateFormat::DER)), + RpcTlsUtilsKeyAndCertTest::PrintParamInfo); } // namespace android diff --git a/libs/binder/tests/binderAllocationLimits.cpp b/libs/binder/tests/binderAllocationLimits.cpp index 7e0b59463a..c0c0aae80b 100644 --- a/libs/binder/tests/binderAllocationLimits.cpp +++ b/libs/binder/tests/binderAllocationLimits.cpp @@ -114,12 +114,12 @@ TEST(TestTheTest, OnMalloc) { { const auto on_malloc = OnMalloc([&](size_t bytes) { mallocs++; - EXPECT_EQ(bytes, 40); + EXPECT_EQ(bytes, 40u); }); imaginary_use = new int[10]; } - EXPECT_EQ(mallocs, 1); + EXPECT_EQ(mallocs, 1u); } @@ -196,9 +196,9 @@ TEST(BinderAllocation, InterfaceDescriptorTransaction) { // Happens to be SM package length. We could switch to forking // and registering our own service if it became an issue. #if defined(__LP64__) - EXPECT_EQ(bytes, 78); + EXPECT_EQ(bytes, 78u); #else - EXPECT_EQ(bytes, 70); + EXPECT_EQ(bytes, 70u); #endif }); @@ -206,7 +206,7 @@ TEST(BinderAllocation, InterfaceDescriptorTransaction) { a_binder->getInterfaceDescriptor(); a_binder->getInterfaceDescriptor(); - EXPECT_EQ(mallocs, 1); + EXPECT_EQ(mallocs, 1u); } TEST(BinderAllocation, SmallTransaction) { @@ -217,11 +217,11 @@ TEST(BinderAllocation, SmallTransaction) { const auto on_malloc = OnMalloc([&](size_t bytes) { mallocs++; // Parcel should allocate a small amount by default - EXPECT_EQ(bytes, 128); + EXPECT_EQ(bytes, 128u); }); manager->checkService(empty_descriptor); - EXPECT_EQ(mallocs, 1); + EXPECT_EQ(mallocs, 1u); } TEST(RpcBinderAllocation, SetupRpcServer) { @@ -250,8 +250,8 @@ TEST(RpcBinderAllocation, SetupRpcServer) { }); ASSERT_EQ(OK, remoteBinder->pingBinder()); } - EXPECT_EQ(mallocs, 1); - EXPECT_EQ(totalBytes, 40); + EXPECT_EQ(mallocs, 1u); + EXPECT_EQ(totalBytes, 40u); } int main(int argc, char** argv) { diff --git a/libs/binder/tests/binderClearBufTest.cpp b/libs/binder/tests/binderClearBufTest.cpp index e43ee5fcf5..3230a3f904 100644 --- a/libs/binder/tests/binderClearBufTest.cpp +++ b/libs/binder/tests/binderClearBufTest.cpp @@ -88,7 +88,7 @@ TEST(BinderClearBuf, ClearKernelBuffer) { // the buffer must have at least some length for the string, but we will // just check it has some length, to avoid assuming anything about the // format - EXPECT_GT(replyBuffer.size(), 0); + EXPECT_GT(replyBuffer.size(), 0u); for (size_t i = 0; i < replyBuffer.size(); i++) { EXPECT_EQ(replyBuffer[i], '0') << "reply buffer at " << i; diff --git a/libs/binder/tests/binderLibTest.cpp b/libs/binder/tests/binderLibTest.cpp index 1f61f1852a..9788d9d1dd 100644 --- a/libs/binder/tests/binderLibTest.cpp +++ b/libs/binder/tests/binderLibTest.cpp @@ -528,8 +528,8 @@ TEST_F(BinderLibTest, Freeze) { EXPECT_EQ(NO_ERROR, IPCThreadState::self()->getProcessFreezeInfo(pid, &sync_received, &async_received)); - EXPECT_EQ(sync_received, 1); - EXPECT_EQ(async_received, 0); + EXPECT_EQ(sync_received, 1u); + EXPECT_EQ(async_received, 0u); EXPECT_EQ(NO_ERROR, IPCThreadState::self()->freeze(pid, false, 0)); EXPECT_EQ(NO_ERROR, m_server->transact(BINDER_LIB_TEST_NOP_TRANSACTION, data, &reply)); @@ -1238,13 +1238,13 @@ TEST_F(BinderLibTest, BufRejected) { data.setDataCapacity(1024); // Write a bogus object at offset 0 to get an entry in the offset table data.writeFileDescriptor(0); - EXPECT_EQ(data.objectsCount(), 1); + EXPECT_EQ(data.objectsCount(), 1u); uint8_t *parcelData = const_cast<uint8_t*>(data.data()); // And now, overwrite it with the buffer object memcpy(parcelData, &obj, sizeof(obj)); data.setDataSize(sizeof(obj)); - EXPECT_EQ(data.objectsCount(), 1); + EXPECT_EQ(data.objectsCount(), 1u); // Either the kernel should reject this transaction (if it's correct), but // if it's not, the server implementation should return an error if it @@ -1269,7 +1269,7 @@ TEST_F(BinderLibTest, WeakRejected) { data.setDataCapacity(1024); // Write a bogus object at offset 0 to get an entry in the offset table data.writeFileDescriptor(0); - EXPECT_EQ(data.objectsCount(), 1); + EXPECT_EQ(data.objectsCount(), 1u); uint8_t *parcelData = const_cast<uint8_t *>(data.data()); // And now, overwrite it with the weak binder memcpy(parcelData, &obj, sizeof(obj)); @@ -1279,7 +1279,7 @@ TEST_F(BinderLibTest, WeakRejected) { // test with an object that libbinder will actually try to release EXPECT_EQ(OK, data.writeStrongBinder(sp<BBinder>::make())); - EXPECT_EQ(data.objectsCount(), 2); + EXPECT_EQ(data.objectsCount(), 2u); // send it many times, since previous error was memory corruption, make it // more likely that the server crashes @@ -1648,8 +1648,8 @@ TEST_P(BinderLibRpcTestP, SetRpcClientDebugNoKeepAliveBinder) { EXPECT_THAT(binder->setRpcClientDebug(std::move(socket), nullptr), Debuggable(StatusEq(UNEXPECTED_NULL))); } -INSTANTIATE_TEST_CASE_P(BinderLibTest, BinderLibRpcTestP, testing::Bool(), - BinderLibRpcTestP::ParamToString); +INSTANTIATE_TEST_SUITE_P(BinderLibTest, BinderLibRpcTestP, testing::Bool(), + BinderLibRpcTestP::ParamToString); class BinderLibTestService : public BBinder { public: diff --git a/libs/binder/tests/binderRpcBenchmark.cpp b/libs/binder/tests/binderRpcBenchmark.cpp index 4f10d7417f..865f0ec9a4 100644 --- a/libs/binder/tests/binderRpcBenchmark.cpp +++ b/libs/binder/tests/binderRpcBenchmark.cpp @@ -216,7 +216,7 @@ void BM_repeatTwoPageString(benchmark::State& state) { // by how many binder calls work together (and by factors like the scheduler, // thermal throttling, core choice, etc..). std::string str = std::string(getpagesize() * 2, 'a'); - CHECK_EQ(str.size(), getpagesize() * 2); + CHECK_EQ(static_cast<ssize_t>(str.size()), getpagesize() * 2); while (state.KeepRunning()) { std::string out; diff --git a/libs/binder/tests/binderRpcTest.cpp b/libs/binder/tests/binderRpcTest.cpp index 2769a88342..c044d3943d 100644 --- a/libs/binder/tests/binderRpcTest.cpp +++ b/libs/binder/tests/binderRpcTest.cpp @@ -64,12 +64,12 @@ constexpr char kTrustyIpcDevice[] = "/dev/trusty-ipc-dev0"; static std::string WaitStatusToString(int wstatus) { if (WIFEXITED(wstatus)) { - return std::format("exit status {}", WEXITSTATUS(wstatus)); + return "exit status " + std::to_string(WEXITSTATUS(wstatus)); } if (WIFSIGNALED(wstatus)) { - return std::format("term signal {}", WTERMSIG(wstatus)); + return "term signal " + std::to_string(WTERMSIG(wstatus)); } - return std::format("unexpected state {}", wstatus); + return "unexpected state " + std::to_string(wstatus); } static void debugBacktrace(pid_t pid) { @@ -177,7 +177,7 @@ public: EXPECT_NE(nullptr, session); EXPECT_NE(nullptr, session->state()); - EXPECT_EQ(0, session->state()->countBinders()) << (session->state()->dump(), "dump:"); + EXPECT_EQ(0u, session->state()->countBinders()) << (session->state()->dump(), "dump:"); wp<RpcSession> weakSession = session; session = nullptr; @@ -235,7 +235,7 @@ static unique_fd connectToUnixBootstrap(const RpcTransportFd& transportFd) { if (binder::os::sendMessageOnSocket(transportFd, &iov, 1, &fds) < 0) { PLOGF("Failed sendMessageOnSocket"); } - return std::move(sockClient); + return sockClient; } #endif // BINDER_RPC_TO_TRUSTY_TEST @@ -263,9 +263,8 @@ std::unique_ptr<ProcessSession> BinderRpc::createRpcTestSocketServerProcessEtc( bool noKernel = GetParam().noKernel; std::string path = GetExecutableDirectory(); - auto servicePath = - std::format("{}/binder_rpc_test_service{}{}", path, - singleThreaded ? "_single_threaded" : "", noKernel ? "_no_kernel" : ""); + auto servicePath = path + "/binder_rpc_test_service" + + (singleThreaded ? "_single_threaded" : "") + (noKernel ? "_no_kernel" : ""); unique_fd bootstrapClientFd, socketFd; @@ -623,7 +622,7 @@ TEST_P(BinderRpc, OnewayCallQueueing) { for (size_t i = 0; i + 1 < kNumQueued; i++) { int n; proc.rootIface->blockingRecvInt(&n); - EXPECT_EQ(n, i); + EXPECT_EQ(n, static_cast<ssize_t>(i)); } saturateThreadPool(1 + kNumExtraServerThreads, proc.rootIface); @@ -1148,8 +1147,8 @@ static std::vector<BinderRpc::ParamType> getTrustyBinderRpcParams() { return ret; } -INSTANTIATE_TEST_CASE_P(Trusty, BinderRpc, ::testing::ValuesIn(getTrustyBinderRpcParams()), - BinderRpc::PrintParamInfo); +INSTANTIATE_TEST_SUITE_P(Trusty, BinderRpc, ::testing::ValuesIn(getTrustyBinderRpcParams()), + BinderRpc::PrintParamInfo); #else // BINDER_RPC_TO_TRUSTY_TEST bool testSupportVsockLoopback() { // We don't need to enable TLS to know if vsock is supported. @@ -1308,8 +1307,8 @@ static std::vector<BinderRpc::ParamType> getBinderRpcParams() { return ret; } -INSTANTIATE_TEST_CASE_P(PerSocket, BinderRpc, ::testing::ValuesIn(getBinderRpcParams()), - BinderRpc::PrintParamInfo); +INSTANTIATE_TEST_SUITE_P(PerSocket, BinderRpc, ::testing::ValuesIn(getBinderRpcParams()), + BinderRpc::PrintParamInfo); class BinderRpcServerRootObject : public ::testing::TestWithParam<std::tuple<bool, bool, RpcSecurity>> {}; @@ -1337,9 +1336,9 @@ TEST_P(BinderRpcServerRootObject, WeakRootObject) { EXPECT_EQ((isStrong2 ? binderRaw2 : nullptr), server->getRootObject()); } -INSTANTIATE_TEST_CASE_P(BinderRpc, BinderRpcServerRootObject, - ::testing::Combine(::testing::Bool(), ::testing::Bool(), - ::testing::ValuesIn(RpcSecurityValues()))); +INSTANTIATE_TEST_SUITE_P(BinderRpc, BinderRpcServerRootObject, + ::testing::Combine(::testing::Bool(), ::testing::Bool(), + ::testing::ValuesIn(RpcSecurityValues()))); class OneOffSignal { public: @@ -1384,7 +1383,7 @@ TEST(BinderRpc, Java) { auto binder = sm->checkService(String16("batteryproperties")); ASSERT_NE(nullptr, binder); auto descriptor = binder->getInterfaceDescriptor(); - ASSERT_GE(descriptor.size(), 0); + ASSERT_GE(descriptor.size(), 0u); ASSERT_EQ(OK, binder->pingBinder()); auto rpcServer = RpcServer::make(); @@ -1468,10 +1467,10 @@ TEST_P(BinderRpcServerOnly, Shutdown) { << "After server->shutdown() returns true, join() did not stop after 2s"; } -INSTANTIATE_TEST_CASE_P(BinderRpc, BinderRpcServerOnly, - ::testing::Combine(::testing::ValuesIn(RpcSecurityValues()), - ::testing::ValuesIn(testVersions())), - BinderRpcServerOnly::PrintTestParam); +INSTANTIATE_TEST_SUITE_P(BinderRpc, BinderRpcServerOnly, + ::testing::Combine(::testing::ValuesIn(RpcSecurityValues()), + ::testing::ValuesIn(testVersions())), + BinderRpcServerOnly::PrintTestParam); class RpcTransportTestUtils { public: @@ -2018,9 +2017,9 @@ TEST_P(RpcTransportTest, CheckWaitingForRead) { server->shutdown(); } -INSTANTIATE_TEST_CASE_P(BinderRpc, RpcTransportTest, - ::testing::ValuesIn(RpcTransportTest::getRpcTranportTestParams()), - RpcTransportTest::PrintParamInfo); +INSTANTIATE_TEST_SUITE_P(BinderRpc, RpcTransportTest, + ::testing::ValuesIn(RpcTransportTest::getRpcTranportTestParams()), + RpcTransportTest::PrintParamInfo); class RpcTransportTlsKeyTest : public testing::TestWithParam< @@ -2075,7 +2074,7 @@ TEST_P(RpcTransportTlsKeyTest, PreSignedCertificate) { client.run(); } -INSTANTIATE_TEST_CASE_P( +INSTANTIATE_TEST_SUITE_P( BinderRpc, RpcTransportTlsKeyTest, testing::Combine(testing::ValuesIn(testSocketTypes(false /* hasPreconnected*/)), testing::Values(RpcCertificateFormat::PEM, RpcCertificateFormat::DER), diff --git a/libs/binder/tests/binderRpcTestCommon.h b/libs/binder/tests/binderRpcTestCommon.h index 8832f1a6ba..acc0373bf4 100644 --- a/libs/binder/tests/binderRpcTestCommon.h +++ b/libs/binder/tests/binderRpcTestCommon.h @@ -60,7 +60,6 @@ #include "../FdUtils.h" #include "../RpcState.h" // for debugging #include "FileUtils.h" -#include "format.h" #include "utils/Errors.h" namespace android { @@ -99,7 +98,7 @@ static inline std::vector<uint32_t> testVersions() { } static inline std::string trustyIpcPort(uint32_t serverVersion) { - return std::format("com.android.trusty.binderRpcTestService.V{}", serverVersion); + return "com.android.trusty.binderRpcTestService.V" + std::to_string(serverVersion); } enum class SocketType { @@ -210,7 +209,7 @@ static inline std::unique_ptr<RpcTransportCtxFactory> newTlsFactory( return RpcTransportCtxFactoryTls::make(std::move(verifier), std::move(auth)); } default: - LOG_ALWAYS_FATAL("Unknown RpcSecurity %d", rpcSecurity); + LOG_ALWAYS_FATAL("Unknown RpcSecurity %d", static_cast<int>(rpcSecurity)); } } @@ -256,7 +255,7 @@ public: mValue.reset(); lock.unlock(); mCvEmpty.notify_all(); - return std::move(v); + return v; } private: diff --git a/libs/binder/tests/binderRpcTestTrusty.cpp b/libs/binder/tests/binderRpcTestTrusty.cpp index 18751cc089..31c0eba1c1 100644 --- a/libs/binder/tests/binderRpcTestTrusty.cpp +++ b/libs/binder/tests/binderRpcTestTrusty.cpp @@ -45,7 +45,7 @@ std::unique_ptr<RpcTransportCtxFactory> BinderRpc::newFactory(RpcSecurity rpcSec case RpcSecurity::RAW: return RpcTransportCtxFactoryTipcTrusty::make(); default: - LOG_ALWAYS_FATAL("Unknown RpcSecurity %d", rpcSecurity); + LOG_ALWAYS_FATAL("Unknown RpcSecurity %d", static_cast<int>(rpcSecurity)); } } @@ -110,8 +110,8 @@ static std::vector<BinderRpc::ParamType> getTrustyBinderRpcParams() { return ret; } -INSTANTIATE_TEST_CASE_P(Trusty, BinderRpc, ::testing::ValuesIn(getTrustyBinderRpcParams()), - BinderRpc::PrintParamInfo); +INSTANTIATE_TEST_SUITE_P(Trusty, BinderRpc, ::testing::ValuesIn(getTrustyBinderRpcParams()), + BinderRpc::PrintParamInfo); } // namespace android diff --git a/libs/binder/tests/format.h b/libs/binder/tests/format.h deleted file mode 100644 index c588de70bd..0000000000 --- a/libs/binder/tests/format.h +++ /dev/null @@ -1,29 +0,0 @@ -/* - * Copyright (C) 2023 The Android Open Source Project - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -// TODO(b/302723053): remove this header and replace with <format> once b/175635923 is done -// ETA for this blocker is 2023-10-27~2023-11-10. -// Also, remember to remove fmtlib's format.cc from trusty makefiles. - -#if __has_include(<format>) && !defined(_LIBCPP_HAS_NO_INCOMPLETE_FORMAT) -#include <format> -#else -#include <fmt/format.h> - -namespace std { -using fmt::format; -} -#endif
\ No newline at end of file diff --git a/libs/gui/Android.bp b/libs/gui/Android.bp index 6c45746335..2547297ff8 100644 --- a/libs/gui/Android.bp +++ b/libs/gui/Android.bp @@ -87,6 +87,7 @@ filegroup { "android/gui/DropInputMode.aidl", "android/gui/StalledTransactionInfo.aidl", "android/**/TouchOcclusionMode.aidl", + "android/gui/TrustedOverlay.aidl", ], } diff --git a/libs/gui/LayerState.cpp b/libs/gui/LayerState.cpp index 0a2879975b..c82bde9a83 100644 --- a/libs/gui/LayerState.cpp +++ b/libs/gui/LayerState.cpp @@ -89,7 +89,7 @@ layer_state_t::layer_state_t() frameRateSelectionStrategy(ANATIVEWINDOW_FRAME_RATE_SELECTION_STRATEGY_PROPAGATE), fixedTransformHint(ui::Transform::ROT_INVALID), autoRefresh(false), - isTrustedOverlay(false), + trustedOverlay(gui::TrustedOverlay::UNSET), bufferCrop(Rect::INVALID_RECT), destinationFrame(Rect::INVALID_RECT), dropInputMode(gui::DropInputMode::NONE) { @@ -179,7 +179,7 @@ status_t layer_state_t::write(Parcel& output) const SAFE_PARCEL(output.write, stretchEffect); SAFE_PARCEL(output.write, bufferCrop); SAFE_PARCEL(output.write, destinationFrame); - SAFE_PARCEL(output.writeBool, isTrustedOverlay); + SAFE_PARCEL(output.writeInt32, static_cast<uint32_t>(trustedOverlay)); SAFE_PARCEL(output.writeUint32, static_cast<uint32_t>(dropInputMode)); @@ -308,7 +308,9 @@ status_t layer_state_t::read(const Parcel& input) SAFE_PARCEL(input.read, stretchEffect); SAFE_PARCEL(input.read, bufferCrop); SAFE_PARCEL(input.read, destinationFrame); - SAFE_PARCEL(input.readBool, &isTrustedOverlay); + uint32_t trustedOverlayInt; + SAFE_PARCEL(input.readUint32, &trustedOverlayInt); + trustedOverlay = static_cast<gui::TrustedOverlay>(trustedOverlayInt); uint32_t mode; SAFE_PARCEL(input.readUint32, &mode); @@ -674,7 +676,7 @@ void layer_state_t::merge(const layer_state_t& other) { } if (other.what & eTrustedOverlayChanged) { what |= eTrustedOverlayChanged; - isTrustedOverlay = other.isTrustedOverlay; + trustedOverlay = other.trustedOverlay; } if (other.what & eStretchChanged) { what |= eStretchChanged; @@ -779,7 +781,7 @@ uint64_t layer_state_t::diff(const layer_state_t& other) const { CHECK_DIFF(diff, eFrameRateSelectionStrategyChanged, other, frameRateSelectionStrategy); CHECK_DIFF(diff, eFixedTransformHintChanged, other, fixedTransformHint); CHECK_DIFF(diff, eAutoRefreshChanged, other, autoRefresh); - CHECK_DIFF(diff, eTrustedOverlayChanged, other, isTrustedOverlay); + CHECK_DIFF(diff, eTrustedOverlayChanged, other, trustedOverlay); CHECK_DIFF(diff, eStretchChanged, other, stretchEffect); CHECK_DIFF(diff, eBufferCropChanged, other, bufferCrop); CHECK_DIFF(diff, eDestinationFrameChanged, other, destinationFrame); diff --git a/libs/gui/SurfaceComposerClient.cpp b/libs/gui/SurfaceComposerClient.cpp index 7aaaebbc8e..b420aabb84 100644 --- a/libs/gui/SurfaceComposerClient.cpp +++ b/libs/gui/SurfaceComposerClient.cpp @@ -1280,18 +1280,22 @@ status_t SurfaceComposerClient::Transaction::sendSurfaceFlushJankDataTransaction } // --------------------------------------------------------------------------- -sp<IBinder> SurfaceComposerClient::createDisplay(const String8& displayName, bool isSecure, - const std::string& uniqueId, - float requestedRefreshRate) { +sp<IBinder> SurfaceComposerClient::createVirtualDisplay(const std::string& displayName, + bool isSecure, const std::string& uniqueId, + float requestedRefreshRate) { sp<IBinder> display = nullptr; - binder::Status status = ComposerServiceAIDL::getComposerService() - ->createDisplay(std::string(displayName.c_str()), isSecure, - uniqueId, requestedRefreshRate, &display); + binder::Status status = + ComposerServiceAIDL::getComposerService()->createVirtualDisplay(displayName, isSecure, + uniqueId, + requestedRefreshRate, + &display); return status.isOk() ? display : nullptr; } -void SurfaceComposerClient::destroyDisplay(const sp<IBinder>& display) { - ComposerServiceAIDL::getComposerService()->destroyDisplay(display); +status_t SurfaceComposerClient::destroyVirtualDisplay(const sp<IBinder>& displayToken) { + return ComposerServiceAIDL::getComposerService() + ->destroyVirtualDisplay(displayToken) + .transactionError(); } std::vector<PhysicalDisplayId> SurfaceComposerClient::getPhysicalDisplayIds() { @@ -2175,6 +2179,13 @@ SurfaceComposerClient::Transaction& SurfaceComposerClient::Transaction::setAutoR SurfaceComposerClient::Transaction& SurfaceComposerClient::Transaction::setTrustedOverlay( const sp<SurfaceControl>& sc, bool isTrustedOverlay) { + return setTrustedOverlay(sc, + isTrustedOverlay ? gui::TrustedOverlay::ENABLED + : gui::TrustedOverlay::UNSET); +} + +SurfaceComposerClient::Transaction& SurfaceComposerClient::Transaction::setTrustedOverlay( + const sp<SurfaceControl>& sc, gui::TrustedOverlay trustedOverlay) { layer_state_t* s = getLayerState(sc); if (!s) { mStatus = BAD_INDEX; @@ -2182,7 +2193,7 @@ SurfaceComposerClient::Transaction& SurfaceComposerClient::Transaction::setTrust } s->what |= layer_state_t::eTrustedOverlayChanged; - s->isTrustedOverlay = isTrustedOverlay; + s->trustedOverlay = trustedOverlay; return *this; } @@ -3114,6 +3125,10 @@ status_t SurfaceComposerClient::removeWindowInfosListener( ->removeWindowInfosListener(windowInfosListener, ComposerServiceAIDL::getComposerService()); } + +void SurfaceComposerClient::notifyShutdown() { + ComposerServiceAIDL::getComposerService()->notifyShutdown(); +} // ---------------------------------------------------------------------------- status_t ScreenshotClient::captureDisplay(const DisplayCaptureArgs& captureArgs, diff --git a/libs/gui/aidl/android/gui/ISurfaceComposer.aidl b/libs/gui/aidl/android/gui/ISurfaceComposer.aidl index c6e7197f24..6d018ea7ef 100644 --- a/libs/gui/aidl/android/gui/ISurfaceComposer.aidl +++ b/libs/gui/aidl/android/gui/ISurfaceComposer.aidl @@ -105,14 +105,14 @@ interface ISurfaceComposer { * * requires ACCESS_SURFACE_FLINGER permission. */ - @nullable IBinder createDisplay(@utf8InCpp String displayName, boolean isSecure, + @nullable IBinder createVirtualDisplay(@utf8InCpp String displayName, boolean isSecure, @utf8InCpp String uniqueId, float requestedRefreshRate); /** * Destroy a virtual display. * requires ACCESS_SURFACE_FLINGER permission. */ - void destroyDisplay(IBinder display); + void destroyVirtualDisplay(IBinder displayToken); /** * Get stable IDs for connected physical displays. @@ -573,4 +573,11 @@ interface ISurfaceComposer { @nullable StalledTransactionInfo getStalledTransactionInfo(int pid); SchedulingPolicy getSchedulingPolicy(); + + /** + * Notifies the SurfaceFlinger that the ShutdownThread is running. When it is called, + * transaction traces will be captured and writted into a file. + * This method should not block the ShutdownThread therefore it's handled asynchronously. + */ + oneway void notifyShutdown(); } diff --git a/libs/gui/android/gui/TrustedOverlay.aidl b/libs/gui/android/gui/TrustedOverlay.aidl new file mode 100644 index 0000000000..06fb5f0bd5 --- /dev/null +++ b/libs/gui/android/gui/TrustedOverlay.aidl @@ -0,0 +1,45 @@ +/** + * Copyright (c) 2024, The Android Open Source Project + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package android.gui; + + +/** + * Trusted overlay state prevents layers from being considered as obscuring for + * input occlusion detection purposes. + * + * @hide + */ +@Backing(type="int") +enum TrustedOverlay { + /** + * The default, layer will inherit the state from its parents. If the parent state is also + * unset, the layer will be considered as untrusted. + */ + UNSET, + + /** + * Treats this layer and all its children as an untrusted overlay. This will override any + * state set by its parent layers. + */ + DISABLED, + + /** + * Treats this layer and all its children as a trusted overlay unless the child layer + * explicitly disables its trusted state. + */ + ENABLED +} diff --git a/libs/gui/include/gui/ISurfaceComposer.h b/libs/gui/include/gui/ISurfaceComposer.h index 738c73a24b..eb4a802c17 100644 --- a/libs/gui/include/gui/ISurfaceComposer.h +++ b/libs/gui/include/gui/ISurfaceComposer.h @@ -130,8 +130,8 @@ public: CREATE_CONNECTION, // Deprecated. Autogenerated by .aidl now. GET_STATIC_DISPLAY_INFO, // Deprecated. Autogenerated by .aidl now. CREATE_DISPLAY_EVENT_CONNECTION, // Deprecated. Autogenerated by .aidl now. - CREATE_DISPLAY, // Deprecated. Autogenerated by .aidl now. - DESTROY_DISPLAY, // Deprecated. Autogenerated by .aidl now. + CREATE_VIRTUAL_DISPLAY, // Deprecated. Autogenerated by .aidl now. + DESTROY_VIRTUAL_DISPLAY, // Deprecated. Autogenerated by .aidl now. GET_PHYSICAL_DISPLAY_TOKEN, // Deprecated. Autogenerated by .aidl now. SET_TRANSACTION_STATE, AUTHENTICATE_SURFACE, // Deprecated. Autogenerated by .aidl now. diff --git a/libs/gui/include/gui/LayerMetadata.h b/libs/gui/include/gui/LayerMetadata.h index 7ee291df4c..9cf62bc7d6 100644 --- a/libs/gui/include/gui/LayerMetadata.h +++ b/libs/gui/include/gui/LayerMetadata.h @@ -74,7 +74,6 @@ enum class GameMode : int32_t { } // namespace android::gui using android::gui::METADATA_ACCESSIBILITY_ID; -using android::gui::METADATA_CALLING_UID; using android::gui::METADATA_DEQUEUE_TIME; using android::gui::METADATA_GAME_MODE; using android::gui::METADATA_MOUSE_CURSOR; diff --git a/libs/gui/include/gui/LayerState.h b/libs/gui/include/gui/LayerState.h index ca7acf9be4..82889efe36 100644 --- a/libs/gui/include/gui/LayerState.h +++ b/libs/gui/include/gui/LayerState.h @@ -30,6 +30,7 @@ #include <android/gui/DropInputMode.h> #include <android/gui/FocusRequest.h> +#include <android/gui/TrustedOverlay.h> #include <ftl/flags.h> #include <gui/DisplayCaptureArgs.h> @@ -385,7 +386,7 @@ struct layer_state_t { // An inherited state that indicates that this surface control and its children // should be trusted for input occlusion detection purposes - bool isTrustedOverlay; + gui::TrustedOverlay trustedOverlay; // Stretch effect to be applied to this layer StretchEffect stretchEffect; diff --git a/libs/gui/include/gui/SurfaceComposerClient.h b/libs/gui/include/gui/SurfaceComposerClient.h index 987efe01ba..9712907396 100644 --- a/libs/gui/include/gui/SurfaceComposerClient.h +++ b/libs/gui/include/gui/SurfaceComposerClient.h @@ -376,11 +376,11 @@ public: sp<SurfaceControl> mirrorDisplay(DisplayId displayId); static const std::string kEmpty; - static sp<IBinder> createDisplay(const String8& displayName, bool isSecure, - const std::string& uniqueId = kEmpty, - float requestedRefreshRate = 0); + static sp<IBinder> createVirtualDisplay(const std::string& displayName, bool isSecure, + const std::string& uniqueId = kEmpty, + float requestedRefreshRate = 0); - static void destroyDisplay(const sp<IBinder>& display); + static status_t destroyVirtualDisplay(const sp<IBinder>& displayToken); static std::vector<PhysicalDisplayId> getPhysicalDisplayIds(); @@ -719,6 +719,8 @@ public: // Sets that this surface control and its children are trusted overlays for input Transaction& setTrustedOverlay(const sp<SurfaceControl>& sc, bool isTrustedOverlay); + Transaction& setTrustedOverlay(const sp<SurfaceControl>& sc, + gui::TrustedOverlay trustedOverlay); // Queues up transactions using this token in SurfaceFlinger. By default, all transactions // from a client are placed on the same queue. This can be used to prevent multiple @@ -824,6 +826,8 @@ public: nullptr); status_t removeWindowInfosListener(const sp<gui::WindowInfosListener>& windowInfosListener); + static void notifyShutdown(); + protected: ReleaseCallbackThread mReleaseCallbackThread; diff --git a/libs/gui/tests/EndToEndNativeInputTest.cpp b/libs/gui/tests/EndToEndNativeInputTest.cpp index 45e33902c7..7d0b512cb4 100644 --- a/libs/gui/tests/EndToEndNativeInputTest.cpp +++ b/libs/gui/tests/EndToEndNativeInputTest.cpp @@ -1184,9 +1184,8 @@ public: MultiDisplayTests() : InputSurfacesTest() { ProcessState::self()->startThreadPool(); } void TearDown() override { - for (auto& token : mVirtualDisplays) { - SurfaceComposerClient::destroyDisplay(token); - } + std::for_each(mVirtualDisplays.begin(), mVirtualDisplays.end(), + SurfaceComposerClient::destroyVirtualDisplay); InputSurfacesTest::TearDown(); } @@ -1201,7 +1200,7 @@ public: std::string name = "VirtualDisplay"; name += std::to_string(mVirtualDisplays.size()); - sp<IBinder> token = SurfaceComposerClient::createDisplay(String8(name.c_str()), isSecure); + sp<IBinder> token = SurfaceComposerClient::createVirtualDisplay(name, isSecure); SurfaceComposerClient::Transaction t; t.setDisplaySurface(token, producer); t.setDisplayFlags(token, receivesInput ? 0x01 /* DisplayDevice::eReceivesInput */ : 0); diff --git a/libs/gui/tests/Surface_test.cpp b/libs/gui/tests/Surface_test.cpp index eee4fb93c9..43cd0f8a7f 100644 --- a/libs/gui/tests/Surface_test.cpp +++ b/libs/gui/tests/Surface_test.cpp @@ -673,13 +673,14 @@ public: return binder::Status::ok(); } - binder::Status createDisplay(const std::string& /*displayName*/, bool /*isSecure*/, - const std::string& /*uniqueId*/, float /*requestedRefreshRate*/, - sp<IBinder>* /*outDisplay*/) override { + binder::Status createVirtualDisplay(const std::string& /*displayName*/, bool /*isSecure*/, + const std::string& /*uniqueId*/, + float /*requestedRefreshRate*/, + sp<IBinder>* /*outDisplay*/) override { return binder::Status::ok(); } - binder::Status destroyDisplay(const sp<IBinder>& /*display*/) override { + binder::Status destroyVirtualDisplay(const sp<IBinder>& /*displayToken*/) override { return binder::Status::ok(); } @@ -984,6 +985,8 @@ public: return binder::Status::ok(); } + binder::Status notifyShutdown() override { return binder::Status::ok(); } + protected: IBinder* onAsBinder() override { return nullptr; } diff --git a/libs/renderengine/include/renderengine/RenderEngine.h b/libs/renderengine/include/renderengine/RenderEngine.h index 980d913d40..7207394356 100644 --- a/libs/renderengine/include/renderengine/RenderEngine.h +++ b/libs/renderengine/include/renderengine/RenderEngine.h @@ -88,6 +88,21 @@ enum class Protection { PROTECTED = 2, }; +// Toggles for skipping or enabling priming of particular shaders. +struct PrimeCacheConfig { + bool cacheHolePunchLayer = true; + bool cacheSolidLayers = true; + bool cacheSolidDimmedLayers = true; + bool cacheImageLayers = true; + bool cacheImageDimmedLayers = true; + bool cacheClippedLayers = true; + bool cacheShadowLayers = true; + bool cachePIPImageLayers = true; + bool cacheTransparentImageDimmedLayers = true; + bool cacheClippedDimmedImageLayers = true; + bool cacheUltraHDR = true; +}; + class RenderEngine { public: enum class ContextPriority { @@ -145,7 +160,7 @@ public: // This interface, while still in use until a suitable replacement is built, // should be considered deprecated, minus some methods which still may be // used to support legacy behavior. - virtual std::future<void> primeCache(bool shouldPrimeUltraHDR) = 0; + virtual std::future<void> primeCache(PrimeCacheConfig config) = 0; // dump the extension strings. always call the base class. virtual void dump(std::string& result) = 0; diff --git a/libs/renderengine/include/renderengine/mock/RenderEngine.h b/libs/renderengine/include/renderengine/mock/RenderEngine.h index a58a65ca9f..a8c242a86f 100644 --- a/libs/renderengine/include/renderengine/mock/RenderEngine.h +++ b/libs/renderengine/include/renderengine/mock/RenderEngine.h @@ -33,7 +33,7 @@ public: RenderEngine(); ~RenderEngine() override; - MOCK_METHOD1(primeCache, std::future<void>(bool)); + MOCK_METHOD1(primeCache, std::future<void>(PrimeCacheConfig)); MOCK_METHOD1(dump, void(std::string&)); MOCK_CONST_METHOD0(getMaxTextureSize, size_t()); MOCK_CONST_METHOD0(getMaxViewportDims, size_t()); diff --git a/libs/renderengine/skia/Cache.cpp b/libs/renderengine/skia/Cache.cpp index abe0d9b0a6..d2468700e4 100644 --- a/libs/renderengine/skia/Cache.cpp +++ b/libs/renderengine/skia/Cache.cpp @@ -630,7 +630,7 @@ static void drawP3ImageLayers(SkiaRenderEngine* renderengine, const DisplaySetti // kFlushAfterEveryLayer = true // in external/skia/src/gpu/gl/builders/GrGLShaderStringBuilder.cpp // gPrintSKSL = true -void Cache::primeShaderCache(SkiaRenderEngine* renderengine, bool shouldPrimeUltraHDR) { +void Cache::primeShaderCache(SkiaRenderEngine* renderengine, PrimeCacheConfig config) { const int previousCount = renderengine->reportShadersCompiled(); if (previousCount) { ALOGD("%d Shaders already compiled before Cache::primeShaderCache ran\n", previousCount); @@ -694,13 +694,24 @@ void Cache::primeShaderCache(SkiaRenderEngine* renderengine, bool shouldPrimeUlt impl::ExternalTexture>(srcBuffer, *renderengine, impl::ExternalTexture::Usage::READABLE | impl::ExternalTexture::Usage::WRITEABLE); - drawHolePunchLayer(renderengine, display, dstTexture); - drawSolidLayers(renderengine, display, dstTexture); - drawSolidLayers(renderengine, p3Display, dstTexture); - drawSolidDimmedLayers(renderengine, display, dstTexture); - drawShadowLayers(renderengine, display, srcTexture); - drawShadowLayers(renderengine, p3Display, srcTexture); + if (config.cacheHolePunchLayer) { + drawHolePunchLayer(renderengine, display, dstTexture); + } + + if (config.cacheSolidLayers) { + drawSolidLayers(renderengine, display, dstTexture); + drawSolidLayers(renderengine, p3Display, dstTexture); + } + + if (config.cacheSolidDimmedLayers) { + drawSolidDimmedLayers(renderengine, display, dstTexture); + } + + if (config.cacheShadowLayers) { + drawShadowLayers(renderengine, display, srcTexture); + drawShadowLayers(renderengine, p3Display, srcTexture); + } if (renderengine->supportsBackgroundBlur()) { drawBlurLayers(renderengine, display, dstTexture); @@ -737,27 +748,40 @@ void Cache::primeShaderCache(SkiaRenderEngine* renderengine, bool shouldPrimeUlt } for (auto texture : textures) { - drawImageLayers(renderengine, display, dstTexture, texture); + if (config.cacheImageLayers) { + drawImageLayers(renderengine, display, dstTexture, texture); + } - drawImageDimmedLayers(renderengine, display, dstTexture, texture); - drawImageDimmedLayers(renderengine, p3Display, dstTexture, texture); - drawImageDimmedLayers(renderengine, bt2020Display, dstTexture, texture); + if (config.cacheImageDimmedLayers) { + drawImageDimmedLayers(renderengine, display, dstTexture, texture); + drawImageDimmedLayers(renderengine, p3Display, dstTexture, texture); + drawImageDimmedLayers(renderengine, bt2020Display, dstTexture, texture); + } - // Draw layers for b/185569240. - drawClippedLayers(renderengine, display, dstTexture, texture); + if (config.cacheClippedLayers) { + // Draw layers for b/185569240. + drawClippedLayers(renderengine, display, dstTexture, texture); + } } - drawPIPImageLayer(renderengine, display, dstTexture, externalTexture); + if (config.cachePIPImageLayers) { + drawPIPImageLayer(renderengine, display, dstTexture, externalTexture); + } - drawTransparentImageDimmedLayers(renderengine, bt2020Display, dstTexture, externalTexture); - drawTransparentImageDimmedLayers(renderengine, display, dstTexture, externalTexture); - drawTransparentImageDimmedLayers(renderengine, p3Display, dstTexture, externalTexture); - drawTransparentImageDimmedLayers(renderengine, p3DisplayEnhance, dstTexture, - externalTexture); + if (config.cacheTransparentImageDimmedLayers) { + drawTransparentImageDimmedLayers(renderengine, bt2020Display, dstTexture, + externalTexture); + drawTransparentImageDimmedLayers(renderengine, display, dstTexture, externalTexture); + drawTransparentImageDimmedLayers(renderengine, p3Display, dstTexture, externalTexture); + drawTransparentImageDimmedLayers(renderengine, p3DisplayEnhance, dstTexture, + externalTexture); + } - drawClippedDimmedImageLayers(renderengine, bt2020Display, dstTexture, externalTexture); + if (config.cacheClippedDimmedImageLayers) { + drawClippedDimmedImageLayers(renderengine, bt2020Display, dstTexture, externalTexture); + } - if (shouldPrimeUltraHDR) { + if (config.cacheUltraHDR) { drawBT2020ClippedImageLayers(renderengine, bt2020Display, dstTexture, externalTexture); drawBT2020ImageLayers(renderengine, bt2020Display, dstTexture, externalTexture); diff --git a/libs/renderengine/skia/Cache.h b/libs/renderengine/skia/Cache.h index 62f6705c89..259432f91c 100644 --- a/libs/renderengine/skia/Cache.h +++ b/libs/renderengine/skia/Cache.h @@ -16,16 +16,21 @@ #pragma once -namespace android::renderengine::skia { +namespace android::renderengine { + +struct PrimeCacheConfig; + +namespace skia { class SkiaRenderEngine; class Cache { public: - static void primeShaderCache(SkiaRenderEngine*, bool shouldPrimeUltraHDR); + static void primeShaderCache(SkiaRenderEngine*, PrimeCacheConfig config); private: Cache() = default; }; -} // namespace android::renderengine::skia +} // namespace skia +} // namespace android::renderengine diff --git a/libs/renderengine/skia/SkiaRenderEngine.cpp b/libs/renderengine/skia/SkiaRenderEngine.cpp index d844764a2f..e62640eb85 100644 --- a/libs/renderengine/skia/SkiaRenderEngine.cpp +++ b/libs/renderengine/skia/SkiaRenderEngine.cpp @@ -246,8 +246,8 @@ namespace skia { using base::StringAppendF; -std::future<void> SkiaRenderEngine::primeCache(bool shouldPrimeUltraHDR) { - Cache::primeShaderCache(this, shouldPrimeUltraHDR); +std::future<void> SkiaRenderEngine::primeCache(PrimeCacheConfig config) { + Cache::primeShaderCache(this, config); return {}; } @@ -731,8 +731,7 @@ void SkiaRenderEngine::drawLayersInternal( [&](const auto& l) { return l.whitePointNits; }); // ...and compute the dimming ratio if dimming is requested - const float displayDimmingRatio = display.targetLuminanceNits > 0.f && - maxLayerWhitePoint > 0.f && display.targetLuminanceNits > maxLayerWhitePoint + const float displayDimmingRatio = display.targetLuminanceNits > 0.f && maxLayerWhitePoint > 0.f ? maxLayerWhitePoint / display.targetLuminanceNits : 1.f; diff --git a/libs/renderengine/skia/SkiaRenderEngine.h b/libs/renderengine/skia/SkiaRenderEngine.h index 38db8100dc..c8f9241257 100644 --- a/libs/renderengine/skia/SkiaRenderEngine.h +++ b/libs/renderengine/skia/SkiaRenderEngine.h @@ -62,7 +62,7 @@ public: SkiaRenderEngine(Threaded, PixelFormat pixelFormat, BlurAlgorithm); ~SkiaRenderEngine() override; - std::future<void> primeCache(bool shouldPrimeUltraHDR) override final; + std::future<void> primeCache(PrimeCacheConfig config) override final; void cleanupPostRender() override final; bool supportsBackgroundBlur() override final { return mBlurFilter != nullptr; diff --git a/libs/renderengine/tests/RenderEngineTest.cpp b/libs/renderengine/tests/RenderEngineTest.cpp index cb8b016e12..4dcaff9ec8 100644 --- a/libs/renderengine/tests/RenderEngineTest.cpp +++ b/libs/renderengine/tests/RenderEngineTest.cpp @@ -3141,7 +3141,9 @@ TEST_P(RenderEngineTest, primeShaderCache) { } initializeRenderEngine(); - auto fut = mRE->primeCache(false); + PrimeCacheConfig config; + config.cacheUltraHDR = false; + auto fut = mRE->primeCache(config); if (fut.valid()) { fut.wait(); } diff --git a/libs/renderengine/tests/RenderEngineThreadedTest.cpp b/libs/renderengine/tests/RenderEngineThreadedTest.cpp index d56dbb2a7b..bdd94023cc 100644 --- a/libs/renderengine/tests/RenderEngineThreadedTest.cpp +++ b/libs/renderengine/tests/RenderEngineThreadedTest.cpp @@ -25,6 +25,7 @@ namespace android { +using renderengine::PrimeCacheConfig; using testing::_; using testing::Eq; using testing::Mock; @@ -48,9 +49,25 @@ TEST_F(RenderEngineThreadedTest, dump) { mThreadedRE->dump(testString); } +MATCHER_P(EqConfig, other, "Equality for prime cache config") { + return arg.cacheHolePunchLayer == other.cacheHolePunchLayer && + arg.cacheSolidLayers == other.cacheSolidLayers && + arg.cacheSolidDimmedLayers == other.cacheSolidDimmedLayers && + arg.cacheImageLayers == other.cacheImageLayers && + arg.cacheImageDimmedLayers == other.cacheImageDimmedLayers && + arg.cacheClippedLayers == other.cacheClippedLayers && + arg.cacheShadowLayers == other.cacheShadowLayers && + arg.cachePIPImageLayers == other.cachePIPImageLayers && + arg.cacheTransparentImageDimmedLayers == other.cacheTransparentImageDimmedLayers && + arg.cacheClippedDimmedImageLayers == other.cacheClippedDimmedImageLayers && + arg.cacheUltraHDR == other.cacheUltraHDR; +} + TEST_F(RenderEngineThreadedTest, primeCache) { - EXPECT_CALL(*mRenderEngine, primeCache(false)); - mThreadedRE->primeCache(false); + PrimeCacheConfig config; + config.cacheUltraHDR = false; + EXPECT_CALL(*mRenderEngine, primeCache(EqConfig(config))); + mThreadedRE->primeCache(config); // need to call ANY synchronous function after primeCache to ensure that primeCache has // completed asynchronously before the test completes execution. mThreadedRE->getContextPriority(); diff --git a/libs/renderengine/threaded/RenderEngineThreaded.cpp b/libs/renderengine/threaded/RenderEngineThreaded.cpp index f4cebc05ec..d27c151e72 100644 --- a/libs/renderengine/threaded/RenderEngineThreaded.cpp +++ b/libs/renderengine/threaded/RenderEngineThreaded.cpp @@ -130,7 +130,7 @@ void RenderEngineThreaded::waitUntilInitialized() const { } } -std::future<void> RenderEngineThreaded::primeCache(bool shouldPrimeUltraHDR) { +std::future<void> RenderEngineThreaded::primeCache(PrimeCacheConfig config) { const auto resultPromise = std::make_shared<std::promise<void>>(); std::future<void> resultFuture = resultPromise->get_future(); ATRACE_CALL(); @@ -138,20 +138,19 @@ std::future<void> RenderEngineThreaded::primeCache(bool shouldPrimeUltraHDR) { // for the futures. { std::lock_guard lock(mThreadMutex); - mFunctionCalls.push( - [resultPromise, shouldPrimeUltraHDR](renderengine::RenderEngine& instance) { - ATRACE_NAME("REThreaded::primeCache"); - if (setSchedFifo(false) != NO_ERROR) { - ALOGW("Couldn't set SCHED_OTHER for primeCache"); - } - - instance.primeCache(shouldPrimeUltraHDR); - resultPromise->set_value(); - - if (setSchedFifo(true) != NO_ERROR) { - ALOGW("Couldn't set SCHED_FIFO for primeCache"); - } - }); + mFunctionCalls.push([resultPromise, config](renderengine::RenderEngine& instance) { + ATRACE_NAME("REThreaded::primeCache"); + if (setSchedFifo(false) != NO_ERROR) { + ALOGW("Couldn't set SCHED_OTHER for primeCache"); + } + + instance.primeCache(config); + resultPromise->set_value(); + + if (setSchedFifo(true) != NO_ERROR) { + ALOGW("Couldn't set SCHED_FIFO for primeCache"); + } + }); } mCondition.notify_one(); diff --git a/libs/renderengine/threaded/RenderEngineThreaded.h b/libs/renderengine/threaded/RenderEngineThreaded.h index d440c961e7..d4997d6c93 100644 --- a/libs/renderengine/threaded/RenderEngineThreaded.h +++ b/libs/renderengine/threaded/RenderEngineThreaded.h @@ -41,7 +41,7 @@ public: RenderEngineThreaded(CreateInstanceFactory factory); ~RenderEngineThreaded() override; - std::future<void> primeCache(bool shouldPrimeUltraHDR) override; + std::future<void> primeCache(PrimeCacheConfig config) override; void dump(std::string& result) override; diff --git a/libs/ui/DisplayIdentification.cpp b/libs/ui/DisplayIdentification.cpp index 0908ae85a8..e5af7406ed 100644 --- a/libs/ui/DisplayIdentification.cpp +++ b/libs/ui/DisplayIdentification.cpp @@ -320,6 +320,11 @@ std::optional<PnpId> getPnpId(PhysicalDisplayId displayId) { std::optional<DisplayIdentificationInfo> parseDisplayIdentificationData( uint8_t port, const DisplayIdentificationData& data) { + if (data.empty()) { + ALOGI("Display identification data is empty."); + return {}; + } + if (!isEdid(data)) { ALOGE("Display identification data has unknown format."); return {}; diff --git a/services/inputflinger/include/InputReaderBase.h b/services/inputflinger/include/InputReaderBase.h index 6cf5a7e1e5..889ee09e6f 100644 --- a/services/inputflinger/include/InputReaderBase.h +++ b/services/inputflinger/include/InputReaderBase.h @@ -106,15 +106,15 @@ struct InputReaderConfiguration { // The associations between input ports and display ports. // Used to determine which DisplayViewport should be tied to which InputDevice. - std::unordered_map<std::string, uint8_t> portAssociations; + std::unordered_map<std::string, uint8_t> inputPortToDisplayPortAssociations; // The associations between input device ports and display unique ids. // Used to determine which DisplayViewport should be tied to which InputDevice. - std::unordered_map<std::string, std::string> uniqueIdAssociationsByPort; + std::unordered_map<std::string, std::string> inputPortToDisplayUniqueIdAssociations; // The associations between input device descriptor and display unique ids. // Used to determine which DisplayViewport should be tied to which InputDevice. - std::unordered_map<std::string, std::string> uniqueIdAssociationsByDescriptor; + std::unordered_map<std::string, std::string> inputDeviceDescriptorToDisplayUniqueIdAssociations; // The associations between input device ports device types. // This is used to determine which device type and source should be tied to which InputDevice. @@ -310,9 +310,6 @@ public: /* Called by the heartbeat to ensures that the reader has not deadlocked. */ virtual void monitor() = 0; - /* Returns true if the input device is enabled. */ - virtual bool isInputDeviceEnabled(int32_t deviceId) = 0; - /* Makes the reader start processing events from the kernel. */ virtual status_t start() = 0; diff --git a/services/inputflinger/reader/InputDevice.cpp b/services/inputflinger/reader/InputDevice.cpp index 956484c931..b807b2714f 100644 --- a/services/inputflinger/reader/InputDevice.cpp +++ b/services/inputflinger/reader/InputDevice.cpp @@ -281,14 +281,14 @@ std::list<NotifyArgs> InputDevice::configureInternal(nsecs_t when, const std::string& inputDeviceDescriptor = mIdentifier.descriptor; if (!inputDeviceDescriptor.empty()) { const std::unordered_map<std::string, uint8_t>& ports = - readerConfig.portAssociations; + readerConfig.inputPortToDisplayPortAssociations; const auto& displayPort = ports.find(inputDeviceDescriptor); if (displayPort != ports.end()) { mAssociatedDisplayPort = std::make_optional(displayPort->second); } else { const std::unordered_map<std::string, std::string>& displayUniqueIdsByDescriptor = - readerConfig.uniqueIdAssociationsByDescriptor; + readerConfig.inputDeviceDescriptorToDisplayUniqueIdAssociations; const auto& displayUniqueIdByDescriptor = displayUniqueIdsByDescriptor.find(inputDeviceDescriptor); if (displayUniqueIdByDescriptor != displayUniqueIdsByDescriptor.end()) { @@ -301,13 +301,13 @@ std::list<NotifyArgs> InputDevice::configureInternal(nsecs_t when, const std::string& inputPort = mIdentifier.location; if (!inputPort.empty()) { const std::unordered_map<std::string, uint8_t>& ports = - readerConfig.portAssociations; + readerConfig.inputPortToDisplayPortAssociations; const auto& displayPort = ports.find(inputPort); if (displayPort != ports.end()) { mAssociatedDisplayPort = std::make_optional(displayPort->second); } else { const std::unordered_map<std::string, std::string>& displayUniqueIdsByPort = - readerConfig.uniqueIdAssociationsByPort; + readerConfig.inputPortToDisplayUniqueIdAssociations; const auto& displayUniqueIdByPort = displayUniqueIdsByPort.find(inputPort); if (displayUniqueIdByPort != displayUniqueIdsByPort.end()) { mAssociatedDisplayUniqueIdByPort = displayUniqueIdByPort->second; diff --git a/services/inputflinger/reader/InputReader.cpp b/services/inputflinger/reader/InputReader.cpp index 69555f8961..b9523ef332 100644 --- a/services/inputflinger/reader/InputReader.cpp +++ b/services/inputflinger/reader/InputReader.cpp @@ -874,17 +874,6 @@ std::optional<std::string> InputReader::getBluetoothAddress(int32_t deviceId) co return std::nullopt; } -bool InputReader::isInputDeviceEnabled(int32_t deviceId) { - std::scoped_lock _l(mLock); - - InputDevice* device = findInputDeviceLocked(deviceId); - if (device) { - return device->isEnabled(); - } - ALOGW("Ignoring invalid device id %" PRId32 ".", deviceId); - return false; -} - bool InputReader::canDispatchToDisplay(int32_t deviceId, ui::LogicalDisplayId displayId) { std::scoped_lock _l(mLock); diff --git a/services/inputflinger/reader/include/InputReader.h b/services/inputflinger/reader/include/InputReader.h index 92a778a5bb..7e701c5341 100644 --- a/services/inputflinger/reader/include/InputReader.h +++ b/services/inputflinger/reader/include/InputReader.h @@ -61,8 +61,6 @@ public: std::vector<InputDeviceInfo> getInputDevices() const override; - bool isInputDeviceEnabled(int32_t deviceId) override; - int32_t getScanCodeState(int32_t deviceId, uint32_t sourceMask, int32_t scanCode) override; int32_t getKeyCodeState(int32_t deviceId, uint32_t sourceMask, int32_t keyCode) override; int32_t getSwitchState(int32_t deviceId, uint32_t sourceMask, int32_t sw) override; diff --git a/services/inputflinger/reader/mapper/MultiTouchInputMapper.cpp b/services/inputflinger/reader/mapper/MultiTouchInputMapper.cpp index 0c58dabd55..bca9d9183b 100644 --- a/services/inputflinger/reader/mapper/MultiTouchInputMapper.cpp +++ b/services/inputflinger/reader/mapper/MultiTouchInputMapper.cpp @@ -138,13 +138,14 @@ void MultiTouchInputMapper::syncTouch(nsecs_t when, RawState* outState) { // Assign pointer id using tracking id if available. if (mHavePointerIds) { - int32_t trackingId = inSlot.getTrackingId(); + const int32_t trackingId = inSlot.getTrackingId(); int32_t id = -1; if (trackingId >= 0) { for (BitSet32 idBits(mPointerIdBits); !idBits.isEmpty();) { uint32_t n = idBits.clearFirstMarkedBit(); if (mPointerTrackingIdMap[n] == trackingId) { id = n; + break; } } diff --git a/services/inputflinger/tests/FakeInputReaderPolicy.cpp b/services/inputflinger/tests/FakeInputReaderPolicy.cpp index 088c7df046..d2cb0ac3df 100644 --- a/services/inputflinger/tests/FakeInputReaderPolicy.cpp +++ b/services/inputflinger/tests/FakeInputReaderPolicy.cpp @@ -129,7 +129,7 @@ void FakeInputReaderPolicy::addExcludedDeviceName(const std::string& deviceName) void FakeInputReaderPolicy::addInputPortAssociation(const std::string& inputPort, uint8_t displayPort) { - mConfig.portAssociations.insert({inputPort, displayPort}); + mConfig.inputPortToDisplayPortAssociations.insert({inputPort, displayPort}); } void FakeInputReaderPolicy::addDeviceTypeAssociation(const std::string& inputPort, @@ -139,7 +139,7 @@ void FakeInputReaderPolicy::addDeviceTypeAssociation(const std::string& inputPor void FakeInputReaderPolicy::addInputUniqueIdAssociation(const std::string& inputUniqueId, const std::string& displayUniqueId) { - mConfig.uniqueIdAssociationsByPort.insert({inputUniqueId, displayUniqueId}); + mConfig.inputPortToDisplayUniqueIdAssociations.insert({inputUniqueId, displayUniqueId}); } void FakeInputReaderPolicy::addKeyboardLayoutAssociation(const std::string& inputUniqueId, diff --git a/services/inputflinger/tests/InputDispatcher_test.cpp b/services/inputflinger/tests/InputDispatcher_test.cpp index 8ad235f3a3..8de28c680f 100644 --- a/services/inputflinger/tests/InputDispatcher_test.cpp +++ b/services/inputflinger/tests/InputDispatcher_test.cpp @@ -7814,6 +7814,7 @@ TEST_F(InputDispatcherTest, MultiDeviceSpyWindowSlipTest) { * If device B reports more ACTION_MOVE events, the middle window should receive remaining events. */ TEST_F(InputDispatcherTest, MultiDeviceSlipperyWindowTest) { + SCOPED_FLAG_OVERRIDE(enable_multi_device_same_window_stream, true); std::shared_ptr<FakeApplicationHandle> application = std::make_shared<FakeApplicationHandle>(); sp<FakeWindowHandle> leftWindow = sp<FakeWindowHandle>::make(application, mDispatcher, "Left window", diff --git a/services/inputflinger/tests/fuzzers/InputReaderFuzzer.cpp b/services/inputflinger/tests/fuzzers/InputReaderFuzzer.cpp index a19726a479..7d26a43440 100644 --- a/services/inputflinger/tests/fuzzers/InputReaderFuzzer.cpp +++ b/services/inputflinger/tests/fuzzers/InputReaderFuzzer.cpp @@ -55,8 +55,6 @@ public: void monitor() { reader->monitor(); } - bool isInputDeviceEnabled(int32_t deviceId) { return reader->isInputDeviceEnabled(deviceId); } - status_t start() { return reader->start(); } status_t stop() { return reader->stop(); } @@ -206,7 +204,6 @@ extern "C" int LLVMFuzzerTestOneInput(uint8_t* data, size_t size) { }, [&]() -> void { reader->monitor(); }, [&]() -> void { reader->getInputDevices(); }, - [&]() -> void { reader->isInputDeviceEnabled(fdp->ConsumeIntegral<int32_t>()); }, [&]() -> void { reader->getScanCodeState(fdp->ConsumeIntegral<int32_t>(), fdp->ConsumeIntegral<uint32_t>(), diff --git a/services/sensorservice/SensorDevice.cpp b/services/sensorservice/SensorDevice.cpp index f62562ce9d..9c4d1ace15 100644 --- a/services/sensorservice/SensorDevice.cpp +++ b/services/sensorservice/SensorDevice.cpp @@ -429,14 +429,18 @@ void SensorDevice::onDynamicSensorsConnected(const std::vector<sensor_t>& dynami } void SensorDevice::onDynamicSensorsDisconnected( - const std::vector<int32_t>& dynamicSensorHandlesRemoved) { - if (sensorservice_flags::sensor_device_on_dynamic_sensor_disconnected()) { - for (auto handle : dynamicSensorHandlesRemoved) { - auto it = mConnectedDynamicSensors.find(handle); - if (it != mConnectedDynamicSensors.end()) { - mConnectedDynamicSensors.erase(it); - } - } + const std::vector<int32_t>& /*dynamicSensorHandlesRemoved*/) { + // This function is currently a no-op has removing data in mConnectedDynamicSensors here will + // cause a race condition between when this callback is invoked and when the dynamic sensor meta + // event is processed by polling. The clean up should only happen after processing the meta + // event. See the call stack of cleanupDisconnectedDynamicSensor. +} + +void SensorDevice::cleanupDisconnectedDynamicSensor(int handle) { + std::lock_guard<std::mutex> lock(mDynamicSensorsMutex); + auto it = mConnectedDynamicSensors.find(handle); + if (it != mConnectedDynamicSensors.end()) { + mConnectedDynamicSensors.erase(it); } } diff --git a/services/sensorservice/SensorDevice.h b/services/sensorservice/SensorDevice.h index 52f7cf2de8..b7b04b5d00 100644 --- a/services/sensorservice/SensorDevice.h +++ b/services/sensorservice/SensorDevice.h @@ -63,6 +63,14 @@ public: std::vector<int32_t> getDynamicSensorHandles(); void handleDynamicSensorConnection(int handle, bool connected); + /** + * Removes handle from connected dynamic sensor list. Note that this method must be called after + * SensorService has done using sensor data. + * + * @param handle of the disconnected dynamic sensor. + */ + void cleanupDisconnectedDynamicSensor(int handle); + status_t initCheck() const; int getHalDeviceVersion() const; diff --git a/services/sensorservice/SensorService.cpp b/services/sensorservice/SensorService.cpp index 69e430901a..31b7f8886c 100644 --- a/services/sensorservice/SensorService.cpp +++ b/services/sensorservice/SensorService.cpp @@ -1273,6 +1273,10 @@ bool SensorService::threadLoop() { } else { int handle = mSensorEventBuffer[i].dynamic_sensor_meta.handle; disconnectDynamicSensor(handle, activeConnections); + if (sensorservice_flags:: + sensor_service_clear_dynamic_sensor_data_at_the_end()) { + device.cleanupDisconnectedDynamicSensor(handle); + } } } } diff --git a/services/sensorservice/senserservice_flags.aconfig b/services/sensorservice/senserservice_flags.aconfig index f20b213823..5b499a8fd8 100644 --- a/services/sensorservice/senserservice_flags.aconfig +++ b/services/sensorservice/senserservice_flags.aconfig @@ -21,3 +21,10 @@ flag { description: "This flag controls we allow to pass in nullptr as scratch in SensorEventConnection::sendEvents()" bug: "339306599" } + +flag { + name: "sensor_service_clear_dynamic_sensor_data_at_the_end" + namespace: "sensors" + description: "When this flag is enabled, sensor service will only erase dynamic sensor data at the end of the threadLoop to prevent race condition." + bug: "329020894" +}
\ No newline at end of file diff --git a/services/surfaceflinger/DisplayDevice.cpp b/services/surfaceflinger/DisplayDevice.cpp index 3bf0eaa0f1..38cf05327f 100644 --- a/services/surfaceflinger/DisplayDevice.cpp +++ b/services/surfaceflinger/DisplayDevice.cpp @@ -485,11 +485,11 @@ void DisplayDevice::enableRefreshRateOverlay(bool enable, bool setByHwc, bool sh } } -void DisplayDevice::updateRefreshRateOverlayRate(Fps vsyncRate, Fps renderFps, bool setByHwc) { +void DisplayDevice::updateRefreshRateOverlayRate(Fps refreshRate, Fps renderFps, bool setByHwc) { ATRACE_CALL(); if (mRefreshRateOverlay) { if (!mRefreshRateOverlay->isSetByHwc() || setByHwc) { - mRefreshRateOverlay->changeRefreshRate(vsyncRate, renderFps); + mRefreshRateOverlay->changeRefreshRate(refreshRate, renderFps); } else { mRefreshRateOverlay->changeRenderRate(renderFps); } diff --git a/services/surfaceflinger/DisplayDevice.h b/services/surfaceflinger/DisplayDevice.h index c2d09c910d..b339bc6c70 100644 --- a/services/surfaceflinger/DisplayDevice.h +++ b/services/surfaceflinger/DisplayDevice.h @@ -226,7 +226,7 @@ public: // Enables an overlay to be displayed with the current refresh rate void enableRefreshRateOverlay(bool enable, bool setByHwc, bool showSpinner, bool showRenderRate, bool showInMiddle) REQUIRES(kMainThreadContext); - void updateRefreshRateOverlayRate(Fps vsyncRate, Fps renderFps, bool setByHwc = false); + void updateRefreshRateOverlayRate(Fps refreshRate, Fps renderFps, bool setByHwc = false); bool isRefreshRateOverlayEnabled() const { return mRefreshRateOverlay != nullptr; } bool onKernelTimerChanged(std::optional<DisplayModeId>, bool timerExpired); diff --git a/services/surfaceflinger/FrameTimeline/FrameTimeline.cpp b/services/surfaceflinger/FrameTimeline/FrameTimeline.cpp index 8369a1ec64..2596a25d15 100644 --- a/services/surfaceflinger/FrameTimeline/FrameTimeline.cpp +++ b/services/surfaceflinger/FrameTimeline/FrameTimeline.cpp @@ -543,12 +543,14 @@ std::string SurfaceFrame::miniDump() const { } void SurfaceFrame::classifyJankLocked(int32_t displayFrameJankType, const Fps& refreshRate, - Fps displayFrameRenderRate, nsecs_t& deadlineDelta) { + Fps displayFrameRenderRate, nsecs_t* outDeadlineDelta) { if (mActuals.presentTime == Fence::SIGNAL_TIME_INVALID) { // Cannot do any classification for invalid present time. mJankType = JankType::Unknown; mJankSeverityType = JankSeverityType::Unknown; - deadlineDelta = -1; + if (outDeadlineDelta) { + *outDeadlineDelta = -1; + } return; } @@ -559,7 +561,9 @@ void SurfaceFrame::classifyJankLocked(int32_t displayFrameJankType, const Fps& r mJankType = mPresentState != PresentState::Presented ? JankType::Dropped : JankType::AppDeadlineMissed; mJankSeverityType = JankSeverityType::Unknown; - deadlineDelta = -1; + if (outDeadlineDelta) { + *outDeadlineDelta = -1; + } return; } @@ -568,11 +572,14 @@ void SurfaceFrame::classifyJankLocked(int32_t displayFrameJankType, const Fps& r return; } - deadlineDelta = mActuals.endTime - mPredictions.endTime; const nsecs_t presentDelta = mActuals.presentTime - mPredictions.presentTime; const nsecs_t deltaToVsync = refreshRate.getPeriodNsecs() > 0 ? std::abs(presentDelta) % refreshRate.getPeriodNsecs() : 0; + const nsecs_t deadlineDelta = mActuals.endTime - mPredictions.endTime; + if (outDeadlineDelta) { + *outDeadlineDelta = deadlineDelta; + } if (deadlineDelta > mJankClassificationThresholds.deadlineThreshold) { mFrameReadyMetadata = FrameReadyMetadata::LateFinish; @@ -671,7 +678,7 @@ void SurfaceFrame::onPresent(nsecs_t presentTime, int32_t displayFrameJankType, mActuals.presentTime = presentTime; nsecs_t deadlineDelta = 0; - classifyJankLocked(displayFrameJankType, refreshRate, displayFrameRenderRate, deadlineDelta); + classifyJankLocked(displayFrameJankType, refreshRate, displayFrameRenderRate, &deadlineDelta); if (mPredictionState != PredictionState::None) { // Only update janky frames if the app used vsync predictions @@ -686,8 +693,7 @@ void SurfaceFrame::onCommitNotComposited(Fps refreshRate, Fps displayFrameRender mDisplayFrameRenderRate = displayFrameRenderRate; mActuals.presentTime = mPredictions.presentTime; - nsecs_t deadlineDelta = 0; - classifyJankLocked(JankType::None, refreshRate, displayFrameRenderRate, deadlineDelta); + classifyJankLocked(JankType::None, refreshRate, displayFrameRenderRate, nullptr); } void SurfaceFrame::tracePredictions(int64_t displayFrameToken, nsecs_t monoBootOffset) const { @@ -1166,7 +1172,7 @@ void FrameTimeline::DisplayFrame::addSkippedFrame(pid_t surfaceFlingerPid, nsecs (static_cast<float>(mSurfaceFlingerPredictions.presentTime) - kThresh * static_cast<float>(mRenderRate.getPeriodNsecs())) && static_cast<float>(surfaceFrame->getPredictions().presentTime) >= - (static_cast<float>(previousPredictionPresentTime) - + (static_cast<float>(previousPredictionPresentTime) + kThresh * static_cast<float>(mRenderRate.getPeriodNsecs())) && // sf skipped frame is not considered if app is self janked !surfaceFrame->isSelfJanky()) { diff --git a/services/surfaceflinger/FrameTimeline/FrameTimeline.h b/services/surfaceflinger/FrameTimeline/FrameTimeline.h index 21bc95a4c5..94cfcb40b9 100644 --- a/services/surfaceflinger/FrameTimeline/FrameTimeline.h +++ b/services/surfaceflinger/FrameTimeline/FrameTimeline.h @@ -237,7 +237,7 @@ private: void tracePredictions(int64_t displayFrameToken, nsecs_t monoBootOffset) const; void traceActuals(int64_t displayFrameToken, nsecs_t monoBootOffset) const; void classifyJankLocked(int32_t displayFrameJankType, const Fps& refreshRate, - Fps displayFrameRenderRate, nsecs_t& deadlineDelta) REQUIRES(mMutex); + Fps displayFrameRenderRate, nsecs_t* outDeadlineDelta) REQUIRES(mMutex); const int64_t mToken; const int32_t mInputEventId; diff --git a/services/surfaceflinger/FrontEnd/LayerSnapshot.h b/services/surfaceflinger/FrontEnd/LayerSnapshot.h index eef8dff94c..398e64a4f1 100644 --- a/services/surfaceflinger/FrontEnd/LayerSnapshot.h +++ b/services/surfaceflinger/FrontEnd/LayerSnapshot.h @@ -84,7 +84,7 @@ struct LayerSnapshot : public compositionengine::LayerFECompositionState { // is a mirror root bool ignoreLocalTransform; gui::DropInputMode dropInputMode; - bool isTrustedOverlay; + gui::TrustedOverlay trustedOverlay; gui::GameMode gameMode; scheduler::LayerInfo::FrameRate frameRate; scheduler::LayerInfo::FrameRate inheritedFrameRate; diff --git a/services/surfaceflinger/FrontEnd/LayerSnapshotBuilder.cpp b/services/surfaceflinger/FrontEnd/LayerSnapshotBuilder.cpp index caeb575c4e..6d4b0b5c54 100644 --- a/services/surfaceflinger/FrontEnd/LayerSnapshotBuilder.cpp +++ b/services/surfaceflinger/FrontEnd/LayerSnapshotBuilder.cpp @@ -22,6 +22,7 @@ #include <numeric> #include <optional> +#include <common/FlagManager.h> #include <ftl/small_map.h> #include <gui/TraceUtils.h> #include <ui/DisplayMap.h> @@ -358,7 +359,7 @@ LayerSnapshot LayerSnapshotBuilder::getRootSnapshot() { snapshot.relativeLayerMetadata.mMap.clear(); snapshot.inputInfo.touchOcclusionMode = gui::TouchOcclusionMode::BLOCK_UNTRUSTED; snapshot.dropInputMode = gui::DropInputMode::NONE; - snapshot.isTrustedOverlay = false; + snapshot.trustedOverlay = gui::TrustedOverlay::UNSET; snapshot.gameMode = gui::GameMode::Unsupported; snapshot.frameRate = {}; snapshot.fixedTransformHint = ui::Transform::ROT_INVALID; @@ -735,7 +736,19 @@ void LayerSnapshotBuilder::updateSnapshot(LayerSnapshot& snapshot, const Args& a } if (forceUpdate || snapshot.clientChanges & layer_state_t::eTrustedOverlayChanged) { - snapshot.isTrustedOverlay = parentSnapshot.isTrustedOverlay || requested.isTrustedOverlay; + switch (requested.trustedOverlay) { + case gui::TrustedOverlay::UNSET: + snapshot.trustedOverlay = parentSnapshot.trustedOverlay; + break; + case gui::TrustedOverlay::DISABLED: + snapshot.trustedOverlay = FlagManager::getInstance().override_trusted_overlay() + ? requested.trustedOverlay + : parentSnapshot.trustedOverlay; + break; + case gui::TrustedOverlay::ENABLED: + snapshot.trustedOverlay = requested.trustedOverlay; + break; + } } if (snapshot.isHiddenByPolicyFromParent && @@ -785,12 +798,10 @@ void LayerSnapshotBuilder::updateSnapshot(LayerSnapshot& snapshot, const Args& a } } - if (forceUpdate || snapshot.changes.test(RequestedLayerState::Changes::Metadata)) { - if (snapshot.changes.test(RequestedLayerState::Changes::GameMode)) { - snapshot.gameMode = requested.metadata.has(gui::METADATA_GAME_MODE) - ? requested.gameMode - : parentSnapshot.gameMode; - } + if (forceUpdate || snapshot.changes.test(RequestedLayerState::Changes::GameMode)) { + snapshot.gameMode = requested.metadata.has(gui::METADATA_GAME_MODE) + ? requested.gameMode + : parentSnapshot.gameMode; updateMetadata(snapshot, requested, args); if (args.includeMetadata) { snapshot.layerMetadata = parentSnapshot.layerMetadata; @@ -1114,7 +1125,7 @@ void LayerSnapshotBuilder::updateInput(LayerSnapshot& snapshot, // Inherit the trusted state from the parent hierarchy, but don't clobber the trusted state // if it was set by WM for a known system overlay - if (snapshot.isTrustedOverlay) { + if (snapshot.trustedOverlay == gui::TrustedOverlay::ENABLED) { snapshot.inputInfo.inputConfig |= InputConfig::TRUSTED_OVERLAY; } diff --git a/services/surfaceflinger/FrontEnd/RequestedLayerState.cpp b/services/surfaceflinger/FrontEnd/RequestedLayerState.cpp index 5631facdae..3e8d74094d 100644 --- a/services/surfaceflinger/FrontEnd/RequestedLayerState.cpp +++ b/services/surfaceflinger/FrontEnd/RequestedLayerState.cpp @@ -118,7 +118,7 @@ RequestedLayerState::RequestedLayerState(const LayerCreationArgs& args) shadowRadius = 0.f; fixedTransformHint = ui::Transform::ROT_INVALID; destinationFrame.makeInvalid(); - isTrustedOverlay = false; + trustedOverlay = gui::TrustedOverlay::UNSET; dropInputMode = gui::DropInputMode::NONE; dimmingEnabled = true; defaultFrameRateCompatibility = static_cast<int8_t>(scheduler::FrameRateCompatibility::Default); @@ -328,7 +328,6 @@ void RequestedLayerState::merge(const ResolvedComposerState& resolvedComposerSta changes |= RequestedLayerState::Changes::GameMode; } } - changes |= RequestedLayerState::Changes::Metadata; } if (clientState.what & layer_state_t::eFrameRateChanged) { const auto compatibility = @@ -588,23 +587,22 @@ bool RequestedLayerState::isSimpleBufferUpdate(const layer_state_t& s) const { const uint64_t deniedFlags = layer_state_t::eProducerDisconnect | layer_state_t::eLayerChanged | layer_state_t::eRelativeLayerChanged | layer_state_t::eTransparentRegionChanged | - layer_state_t::eFlagsChanged | layer_state_t::eBlurRegionsChanged | - layer_state_t::eLayerStackChanged | layer_state_t::eReparent | + layer_state_t::eBlurRegionsChanged | layer_state_t::eLayerStackChanged | + layer_state_t::eReparent | (FlagManager::getInstance().latch_unsignaled_with_auto_refresh_changed() ? 0 - : layer_state_t::eAutoRefreshChanged); + : (layer_state_t::eAutoRefreshChanged | layer_state_t::eFlagsChanged)); if (s.what & deniedFlags) { ATRACE_FORMAT_INSTANT("%s: false [has denied flags 0x%" PRIx64 "]", __func__, s.what & deniedFlags); return false; } - bool changedFlags = diff(s); - static constexpr auto deniedChanges = layer_state_t::ePositionChanged | - layer_state_t::eAlphaChanged | layer_state_t::eColorTransformChanged | - layer_state_t::eBackgroundColorChanged | layer_state_t::eMatrixChanged | - layer_state_t::eCornerRadiusChanged | layer_state_t::eBackgroundBlurRadiusChanged | - layer_state_t::eBufferTransformChanged | + const uint64_t changedFlags = diff(s); + const uint64_t deniedChanges = layer_state_t::ePositionChanged | layer_state_t::eAlphaChanged | + layer_state_t::eColorTransformChanged | layer_state_t::eBackgroundColorChanged | + layer_state_t::eMatrixChanged | layer_state_t::eCornerRadiusChanged | + layer_state_t::eBackgroundBlurRadiusChanged | layer_state_t::eBufferTransformChanged | layer_state_t::eTransformToDisplayInverseChanged | layer_state_t::eCropChanged | layer_state_t::eDataspaceChanged | layer_state_t::eHdrMetadataChanged | layer_state_t::eSidebandStreamChanged | layer_state_t::eColorSpaceAgnosticChanged | @@ -612,10 +610,13 @@ bool RequestedLayerState::isSimpleBufferUpdate(const layer_state_t& s) const { layer_state_t::eTrustedOverlayChanged | layer_state_t::eStretchChanged | layer_state_t::eBufferCropChanged | layer_state_t::eDestinationFrameChanged | layer_state_t::eDimmingEnabledChanged | layer_state_t::eExtendedRangeBrightnessChanged | - layer_state_t::eDesiredHdrHeadroomChanged; + layer_state_t::eDesiredHdrHeadroomChanged | + (FlagManager::getInstance().latch_unsignaled_with_auto_refresh_changed() + ? layer_state_t::eFlagsChanged + : 0); if (changedFlags & deniedChanges) { ATRACE_FORMAT_INSTANT("%s: false [has denied changes flags 0x%" PRIx64 "]", __func__, - s.what & deniedChanges); + changedFlags & deniedChanges); return false; } diff --git a/services/surfaceflinger/FrontEnd/RequestedLayerState.h b/services/surfaceflinger/FrontEnd/RequestedLayerState.h index 09f33de63b..48b9640486 100644 --- a/services/surfaceflinger/FrontEnd/RequestedLayerState.h +++ b/services/surfaceflinger/FrontEnd/RequestedLayerState.h @@ -26,6 +26,7 @@ #include "TransactionState.h" namespace android::surfaceflinger::frontend { +using namespace ftl::flag_operators; // Stores client requested states for a layer. // This struct does not store any other states or states pertaining to @@ -58,6 +59,13 @@ struct RequestedLayerState : layer_state_t { GameMode = 1u << 19, BufferUsageFlags = 1u << 20, }; + + static constexpr ftl::Flags<Changes> kMustComposite = Changes::Created | Changes::Destroyed | + Changes::Hierarchy | Changes::Geometry | Changes::Content | Changes::Input | + Changes::Z | Changes::Mirror | Changes::Parent | Changes::RelativeParent | + Changes::Metadata | Changes::Visibility | Changes::VisibleRegion | Changes::Buffer | + Changes::SidebandStream | Changes::Animation | Changes::BufferSize | Changes::GameMode | + Changes::BufferUsageFlags; static Rect reduce(const Rect& win, const Region& exclude); RequestedLayerState(const LayerCreationArgs&); void merge(const ResolvedComposerState&); diff --git a/services/surfaceflinger/Layer.cpp b/services/surfaceflinger/Layer.cpp index 363b35c4f3..6b97e2f799 100644 --- a/services/surfaceflinger/Layer.cpp +++ b/services/surfaceflinger/Layer.cpp @@ -3902,7 +3902,7 @@ bool Layer::isSimpleBufferUpdate(const layer_state_t& s) const { } if (s.what & layer_state_t::eTrustedOverlayChanged) { - if (mDrawingState.isTrustedOverlay != s.isTrustedOverlay) { + if (mDrawingState.isTrustedOverlay != (s.trustedOverlay == gui::TrustedOverlay::ENABLED)) { ATRACE_FORMAT_INSTANT("%s: false [eTrustedOverlayChanged changed]", __func__); return false; } diff --git a/services/surfaceflinger/LayerProtoHelper.cpp b/services/surfaceflinger/LayerProtoHelper.cpp index 753886ad24..496033b749 100644 --- a/services/surfaceflinger/LayerProtoHelper.cpp +++ b/services/surfaceflinger/LayerProtoHelper.cpp @@ -382,7 +382,8 @@ void LayerProtoHelper::writeSnapshotToProto(perfetto::protos::LayerProto* layerI layerInfo->set_corner_radius( (snapshot.roundedCorner.radius.x + snapshot.roundedCorner.radius.y) / 2.0); layerInfo->set_background_blur_radius(snapshot.backgroundBlurRadius); - layerInfo->set_is_trusted_overlay(snapshot.isTrustedOverlay); + layerInfo->set_is_trusted_overlay(snapshot.trustedOverlay == gui::TrustedOverlay::ENABLED); + // TODO(b/339701674) update protos LayerProtoHelper::writeToProtoDeprecated(transform, layerInfo->mutable_transform()); LayerProtoHelper::writePositionToProto(transform.tx(), transform.ty(), [&]() { return layerInfo->mutable_position(); }); diff --git a/services/surfaceflinger/RefreshRateOverlay.cpp b/services/surfaceflinger/RefreshRateOverlay.cpp index b960e33682..b40f3323bc 100644 --- a/services/surfaceflinger/RefreshRateOverlay.cpp +++ b/services/surfaceflinger/RefreshRateOverlay.cpp @@ -28,7 +28,7 @@ namespace android { -auto RefreshRateOverlay::draw(int vsyncRate, int renderFps, SkColor color, +auto RefreshRateOverlay::draw(int refreshRate, int renderFps, SkColor color, ui::Transform::RotationFlags rotation, ftl::Flags<Features> features) -> Buffers { const size_t loopCount = features.test(Features::Spinner) ? 6 : 1; @@ -71,7 +71,7 @@ auto RefreshRateOverlay::draw(int vsyncRate, int renderFps, SkColor color, canvas->setMatrix(canvasTransform); int left = 0; - drawNumber(vsyncRate, left, color, *canvas); + drawNumber(refreshRate, left, color, *canvas); left += 3 * (kDigitWidth + kDigitSpace); if (features.test(Features::Spinner)) { switch (i) { @@ -171,7 +171,7 @@ bool RefreshRateOverlay::initCheck() const { return mSurfaceControl != nullptr; } -auto RefreshRateOverlay::getOrCreateBuffers(Fps vsyncRate, Fps renderFps) -> const Buffers& { +auto RefreshRateOverlay::getOrCreateBuffers(Fps refreshRate, Fps renderFps) -> const Buffers& { static const Buffers kNoBuffers; if (!mSurfaceControl) return kNoBuffers; @@ -198,16 +198,16 @@ auto RefreshRateOverlay::getOrCreateBuffers(Fps vsyncRate, Fps renderFps) -> con createTransaction().setTransform(mSurfaceControl->get(), transform).apply(); BufferCache::const_iterator it = - mBufferCache.find({vsyncRate.getIntValue(), renderFps.getIntValue(), transformHint}); + mBufferCache.find({refreshRate.getIntValue(), renderFps.getIntValue(), transformHint}); if (it == mBufferCache.end()) { // HWC minFps is not known by the framework in order // to consider lower rates we set minFps to 0. const int minFps = isSetByHwc() ? 0 : mFpsRange.min.getIntValue(); const int maxFps = mFpsRange.max.getIntValue(); - // Clamp to the range. The current vsyncRate may be outside of this range if the display + // Clamp to the range. The current refreshRate may be outside of this range if the display // has changed its set of supported refresh rates. - const int displayIntFps = std::clamp(vsyncRate.getIntValue(), minFps, maxFps); + const int displayIntFps = std::clamp(refreshRate.getIntValue(), minFps, maxFps); const int renderIntFps = renderFps.getIntValue(); // Ensure non-zero range to avoid division by zero. @@ -260,25 +260,26 @@ void RefreshRateOverlay::setLayerStack(ui::LayerStack stack) { createTransaction().setLayerStack(mSurfaceControl->get(), stack).apply(); } -void RefreshRateOverlay::changeRefreshRate(Fps vsyncRate, Fps renderFps) { - mVsyncRate = vsyncRate; +void RefreshRateOverlay::changeRefreshRate(Fps refreshRate, Fps renderFps) { + mRefreshRate = refreshRate; mRenderFps = renderFps; - const auto buffer = getOrCreateBuffers(vsyncRate, renderFps)[mFrame]; + const auto buffer = getOrCreateBuffers(refreshRate, renderFps)[mFrame]; createTransaction().setBuffer(mSurfaceControl->get(), buffer).apply(); } void RefreshRateOverlay::changeRenderRate(Fps renderFps) { - if (mFeatures.test(Features::RenderRate) && mVsyncRate && FlagManager::getInstance().misc1()) { + if (mFeatures.test(Features::RenderRate) && mRefreshRate && + FlagManager::getInstance().misc1()) { mRenderFps = renderFps; - const auto buffer = getOrCreateBuffers(*mVsyncRate, renderFps)[mFrame]; + const auto buffer = getOrCreateBuffers(*mRefreshRate, renderFps)[mFrame]; createTransaction().setBuffer(mSurfaceControl->get(), buffer).apply(); } } void RefreshRateOverlay::animate() { - if (!mFeatures.test(Features::Spinner) || !mVsyncRate) return; + if (!mFeatures.test(Features::Spinner) || !mRefreshRate) return; - const auto& buffers = getOrCreateBuffers(*mVsyncRate, *mRenderFps); + const auto& buffers = getOrCreateBuffers(*mRefreshRate, *mRenderFps); mFrame = (mFrame + 1) % buffers.size(); const auto buffer = buffers[mFrame]; createTransaction().setBuffer(mSurfaceControl->get(), buffer).apply(); diff --git a/services/surfaceflinger/RefreshRateOverlay.h b/services/surfaceflinger/RefreshRateOverlay.h index 0fec470edc..93ec36e7f8 100644 --- a/services/surfaceflinger/RefreshRateOverlay.h +++ b/services/surfaceflinger/RefreshRateOverlay.h @@ -74,12 +74,12 @@ private: SurfaceComposerClient::Transaction createTransaction() const; struct Key { - int vsyncRate; + int refreshRate; int renderFps; ui::Transform::RotationFlags flags; bool operator==(Key other) const { - return vsyncRate == other.vsyncRate && renderFps == other.renderFps && + return refreshRate == other.refreshRate && renderFps == other.renderFps && flags == other.flags; } }; @@ -87,7 +87,7 @@ private: using BufferCache = ftl::SmallMap<Key, Buffers, 9>; BufferCache mBufferCache; - std::optional<Fps> mVsyncRate; + std::optional<Fps> mRefreshRate; std::optional<Fps> mRenderFps; size_t mFrame = 0; diff --git a/services/surfaceflinger/Scheduler/Scheduler.cpp b/services/surfaceflinger/Scheduler/Scheduler.cpp index f72e7a05d5..60681a2bc8 100644 --- a/services/surfaceflinger/Scheduler/Scheduler.cpp +++ b/services/surfaceflinger/Scheduler/Scheduler.cpp @@ -1153,8 +1153,10 @@ auto Scheduler::chooseDisplayModes() const -> DisplayModeChoiceMap { return pacesetterFps; }(); + // Choose a mode for powered-on follower displays. for (const auto& [id, display] : mDisplays) { if (id == *mPacesetterDisplayId) continue; + if (display.powerMode != hal::PowerMode::ON) continue; auto rankedFrameRates = display.selectorPtr->getRankedFrameRates(mPolicy.contentRequirements, globalSignals, diff --git a/services/surfaceflinger/SurfaceFlinger.cpp b/services/surfaceflinger/SurfaceFlinger.cpp index 95f37997ef..5f81cd45cc 100644 --- a/services/surfaceflinger/SurfaceFlinger.cpp +++ b/services/surfaceflinger/SurfaceFlinger.cpp @@ -56,6 +56,7 @@ #include <configstore/Utils.h> #include <cutils/compiler.h> #include <cutils/properties.h> +#include <fmt/format.h> #include <ftl/algorithm.h> #include <ftl/concat.h> #include <ftl/fake_guard.h> @@ -573,8 +574,9 @@ void SurfaceFlinger::run() { mScheduler->run(); } -sp<IBinder> SurfaceFlinger::createDisplay(const String8& displayName, bool isSecure, - const std::string& uniqueId, float requestedRefreshRate) { +sp<IBinder> SurfaceFlinger::createVirtualDisplay(const std::string& displayName, bool isSecure, + const std::string& uniqueId, + float requestedRefreshRate) { // SurfaceComposerAIDL checks for some permissions, but adding an additional check here. // This is to ensure that only root, system, and graphics can request to create a secure // display. Secure displays can show secure content so we add an additional restriction on it. @@ -614,22 +616,23 @@ sp<IBinder> SurfaceFlinger::createDisplay(const String8& displayName, bool isSec return token; } -void SurfaceFlinger::destroyDisplay(const sp<IBinder>& displayToken) { +status_t SurfaceFlinger::destroyVirtualDisplay(const sp<IBinder>& displayToken) { Mutex::Autolock lock(mStateLock); const ssize_t index = mCurrentState.displays.indexOfKey(displayToken); if (index < 0) { ALOGE("%s: Invalid display token %p", __func__, displayToken.get()); - return; + return NAME_NOT_FOUND; } const DisplayDeviceState& state = mCurrentState.displays.valueAt(index); if (state.physical) { ALOGE("%s: Invalid operation on physical display", __func__); - return; + return INVALID_OPERATION; } mCurrentState.displays.removeItemsAt(index); setTransactionFlags(eDisplayTransactionNeeded); + return NO_ERROR; } void SurfaceFlinger::enableHalVirtualDisplays(bool enable) { @@ -941,9 +944,34 @@ void SurfaceFlinger::init() FTL_FAKE_GUARD(kMainThreadContext) { } mRenderEnginePrimeCacheFuture.callOnce([this] { - const bool shouldPrimeUltraHDR = + renderengine::PrimeCacheConfig config; + config.cacheHolePunchLayer = + base::GetBoolProperty("debug.sf.prime_shader_cache.hole_punch"s, true); + config.cacheSolidLayers = + base::GetBoolProperty("debug.sf.prime_shader_cache.solid_layers"s, true); + config.cacheSolidDimmedLayers = + base::GetBoolProperty("debug.sf.prime_shader_cache.solid_dimmed_layers"s, true); + config.cacheImageLayers = + base::GetBoolProperty("debug.sf.prime_shader_cache.image_layers"s, true); + config.cacheImageDimmedLayers = + base::GetBoolProperty("debug.sf.prime_shader_cache.image_dimmed_layers"s, true); + config.cacheClippedLayers = + base::GetBoolProperty("debug.sf.prime_shader_cache.clipped_layers"s, true); + config.cacheShadowLayers = + base::GetBoolProperty("debug.sf.prime_shader_cache.shadow_layers"s, true); + config.cachePIPImageLayers = + base::GetBoolProperty("debug.sf.prime_shader_cache.pip_image_layers"s, true); + config.cacheTransparentImageDimmedLayers = base:: + GetBoolProperty("debug.sf.prime_shader_cache.transparent_image_dimmed_layers"s, + true); + config.cacheClippedDimmedImageLayers = base:: + GetBoolProperty("debug.sf.prime_shader_cache.clipped_dimmed_image_layers"s, + true); + // ro.surface_flinger.prime_chader_cache.ultrahdr exists as a previous ro property + // which we maintain for backwards compatibility. + config.cacheUltraHDR = base::GetBoolProperty("ro.surface_flinger.prime_shader_cache.ultrahdr"s, false); - return getRenderEngine().primeCache(shouldPrimeUltraHDR); + return getRenderEngine().primeCache(config); }); if (setSchedFifo(true) != NO_ERROR) { @@ -985,8 +1013,9 @@ void SurfaceFlinger::initTransactionTraceWriter() { ALOGD("TransactionTraceWriter: file=%s already exists", filename.c_str()); return; } - mTransactionTracing->flush(); + ALOGD("TransactionTraceWriter: writing file=%s", filename.c_str()); mTransactionTracing->writeToFile(filename); + mTransactionTracing->flush(); }; if (std::this_thread::get_id() == mMainThreadId) { writeFn(); @@ -1425,11 +1454,6 @@ void SurfaceFlinger::initiateDisplayModeChanges() { continue; } - if (!shouldApplyRefreshRateSelectorPolicy(*display)) { - dropModeRequest(display); - continue; - } - const auto desiredModeId = desiredModeOpt->mode.modePtr->getId(); const auto displayModePtrOpt = physical.snapshot().displayModes().get(desiredModeId); @@ -2231,12 +2255,12 @@ void SurfaceFlinger::onRefreshRateChangedDebug(const RefreshRateChangedDebugData kMainThreadContext) { if (const auto displayIdOpt = getHwComposer().toPhysicalDisplayId(data.display)) { if (const auto display = getDisplayDeviceLocked(*displayIdOpt)) { - const Fps fps = Fps::fromPeriodNsecs(getHwComposer().getComposer()->isVrrSupported() - ? data.refreshPeriodNanos - : data.vsyncPeriodNanos); - ATRACE_FORMAT("%s Fps %d", whence, fps.getIntValue()); - display->updateRefreshRateOverlayRate(fps, display->getActiveMode().fps, - /* setByHwc */ true); + const Fps refreshRate = Fps::fromPeriodNsecs( + getHwComposer().getComposer()->isVrrSupported() ? data.refreshPeriodNanos + : data.vsyncPeriodNanos); + ATRACE_FORMAT("%s refresh rate = %d", whence, refreshRate.getIntValue()); + display->updateRefreshRateOverlayRate(refreshRate, display->getActiveMode().fps, + /* showRefreshRate */ true); } } })); @@ -2439,7 +2463,12 @@ bool SurfaceFlinger::updateLayerSnapshots(VsyncId vsyncId, nsecs_t frameTimeNs, mUpdateAttachedChoreographer = true; } outTransactionsAreEmpty = mLayerLifecycleManager.getGlobalChanges().get() == 0; - mustComposite |= mLayerLifecycleManager.getGlobalChanges().get() != 0; + if (FlagManager::getInstance().vrr_bugfix_24q4()) { + mustComposite |= mLayerLifecycleManager.getGlobalChanges().any( + frontend::RequestedLayerState::kMustComposite); + } else { + mustComposite |= mLayerLifecycleManager.getGlobalChanges().get() != 0; + } bool newDataLatched = false; ATRACE_NAME("DisplayCallbackAndStatsUpdates"); @@ -4228,12 +4257,6 @@ void SurfaceFlinger::requestDisplayModes(std::vector<display::DisplayModeRequest if (!display) continue; - if (ftl::FakeGuard guard(kMainThreadContext); - !shouldApplyRefreshRateSelectorPolicy(*display)) { - ALOGV("%s(%s): Skipped applying policy", __func__, to_string(displayId).c_str()); - continue; - } - if (display->refreshRateSelector().isModeAllowed(request.mode)) { setDesiredMode(std::move(request)); } else { @@ -5688,7 +5711,7 @@ uint32_t SurfaceFlinger::setClientStateLocked(const FrameTimelineInfo& frameTime if (layer->setHdrMetadata(s.hdrMetadata)) flags |= eTraversalNeeded; } if (what & layer_state_t::eTrustedOverlayChanged) { - if (layer->setTrustedOverlay(s.isTrustedOverlay)) { + if (layer->setTrustedOverlay(s.trustedOverlay == gui::TrustedOverlay::ENABLED)) { flags |= eTraversalNeeded; } } @@ -6499,12 +6522,17 @@ void SurfaceFlinger::dumpDisplayIdentificationData(std::string& result) const { uint8_t port; DisplayIdentificationData data; if (!getHwComposer().getDisplayIdentificationData(*hwcDisplayId, &port, &data)) { - result.append("no identification data\n"); + result.append("no display identification data\n"); + continue; + } + + if (data.empty()) { + result.append("empty display identification data\n"); continue; } if (!isEdid(data)) { - result.append("unknown identification data\n"); + result.append("unknown format for display identification data\n"); continue; } @@ -6537,18 +6565,19 @@ void SurfaceFlinger::dumpWideColorInfo(std::string& result) const { StringAppendF(&result, "DisplayColorSetting: %s\n", decodeDisplayColorSetting(mDisplayColorSetting).c_str()); - // TODO: print out if wide-color mode is active or not + // TODO: print out if wide-color mode is active or not. for (const auto& [id, display] : mPhysicalDisplays) { StringAppendF(&result, "Display %s color modes:\n", to_string(id).c_str()); for (const auto mode : display.snapshot().colorModes()) { - StringAppendF(&result, " %s (%d)\n", decodeColorMode(mode).c_str(), mode); + StringAppendF(&result, " %s (%d)\n", decodeColorMode(mode).c_str(), + fmt::underlying(mode)); } if (const auto display = getDisplayDeviceLocked(id)) { ui::ColorMode currentMode = display->getCompositionDisplay()->getState().colorMode; StringAppendF(&result, " Current color mode: %s (%d)\n", - decodeColorMode(currentMode).c_str(), currentMode); + decodeColorMode(currentMode).c_str(), fmt::underlying(currentMode)); } } result.append("\n"); @@ -7005,8 +7034,8 @@ status_t SurfaceFlinger::CheckTransactCodeCredentials(uint32_t code) { // Used by apps to hook Choreographer to SurfaceFlinger. case CREATE_DISPLAY_EVENT_CONNECTION: case CREATE_CONNECTION: - case CREATE_DISPLAY: - case DESTROY_DISPLAY: + case CREATE_VIRTUAL_DISPLAY: + case DESTROY_VIRTUAL_DISPLAY: case GET_PRIMARY_PHYSICAL_DISPLAY_ID: case GET_PHYSICAL_DISPLAY_IDS: case GET_PHYSICAL_DISPLAY_TOKEN: @@ -8575,35 +8604,11 @@ status_t SurfaceFlinger::setDesiredDisplayModeSpecsInternal( break; } - if (!shouldApplyRefreshRateSelectorPolicy(*display)) { - ALOGV("%s(%s): Skipped applying policy", __func__, to_string(displayId).c_str()); - return NO_ERROR; - } - return applyRefreshRateSelectorPolicy(displayId, selector); } -bool SurfaceFlinger::shouldApplyRefreshRateSelectorPolicy(const DisplayDevice& display) const { - if (display.isPoweredOn() || mPhysicalDisplays.size() == 1) return true; - - LOG_ALWAYS_FATAL_IF(display.isVirtual()); - const auto displayId = display.getPhysicalId(); - - // The display is powered off, and this is a multi-display device. If the display is the - // inactive internal display of a dual-display foldable, then the policy will be applied - // when it becomes active upon powering on. - // - // TODO(b/255635711): Remove this function (i.e. returning `false` as a special case) once - // concurrent mode setting across multiple (potentially powered off) displays is supported. - // - return displayId == mActiveDisplayId || - !mPhysicalDisplays.get(displayId) - .transform(&PhysicalDisplay::isInternal) - .value_or(false); -} - status_t SurfaceFlinger::applyRefreshRateSelectorPolicy( - PhysicalDisplayId displayId, const scheduler::RefreshRateSelector& selector, bool force) { + PhysicalDisplayId displayId, const scheduler::RefreshRateSelector& selector) { const scheduler::RefreshRateSelector::Policy currentPolicy = selector.getCurrentPolicy(); ALOGV("Setting desired display mode specs: %s", currentPolicy.toString().c_str()); @@ -8634,7 +8639,7 @@ status_t SurfaceFlinger::applyRefreshRateSelectorPolicy( return INVALID_OPERATION; } - setDesiredMode({std::move(preferredMode), .emitEvent = true, .force = force}); + setDesiredMode({std::move(preferredMode), .emitEvent = true}); // Update the frameRateOverride list as the display render rate might have changed if (mScheduler->updateFrameRateOverrides(scheduler::GlobalSignals{}, preferredFps)) { @@ -8846,7 +8851,8 @@ void SurfaceFlinger::enableRefreshRateOverlay(bool enable) { const auto status = getHwComposer().setRefreshRateChangedCallbackDebugEnabled(id, enable); if (status != NO_ERROR) { - ALOGE("Error updating the refresh rate changed callback debug enabled"); + ALOGE("Error %s refresh rate changed callback debug", + enable ? "enabling" : "disabling"); enableOverlay(/*setByHwc*/ false); } } @@ -8992,13 +8998,8 @@ void SurfaceFlinger::onActiveDisplayChangedLocked(const DisplayDevice* inactiveD const DisplayDevice& activeDisplay) { ATRACE_CALL(); - // For the first display activated during boot, there is no need to force setDesiredMode, - // because DM is about to send its policy via setDesiredDisplayModeSpecs. - bool forceApplyPolicy = false; - if (inactiveDisplayPtr) { inactiveDisplayPtr->getCompositionDisplay()->setLayerCachingTexturePoolEnabled(false); - forceApplyPolicy = true; } mActiveDisplayId = activeDisplay.getPhysicalId(); @@ -9015,12 +9016,11 @@ void SurfaceFlinger::onActiveDisplayChangedLocked(const DisplayDevice* inactiveD mActiveDisplayTransformHint = activeDisplay.getTransformHint(); sActiveDisplayRotationFlags = ui::Transform::toRotationFlags(activeDisplay.getOrientation()); - // The policy of the new active/pacesetter display may have changed while it was inactive. In - // that case, its preferred mode has not been propagated to HWC (via setDesiredMode). In either - // case, the Scheduler's cachedModeChangedParams must be initialized to the newly active mode, - // and the kernel idle timer of the newly active display must be toggled. - applyRefreshRateSelectorPolicy(mActiveDisplayId, activeDisplay.refreshRateSelector(), - forceApplyPolicy); + // Whether or not the policy of the new active/pacesetter display changed while it was inactive + // (in which case its preferred mode has already been propagated to HWC via setDesiredMode), the + // Scheduler's cachedModeChangedParams must be initialized to the newly active mode, and the + // kernel idle timer of the newly active display must be toggled. + applyRefreshRateSelectorPolicy(mActiveDisplayId, activeDisplay.refreshRateSelector()); } status_t SurfaceFlinger::addWindowInfosListener(const sp<IWindowInfosListener>& windowInfosListener, @@ -9565,26 +9565,25 @@ binder::Status SurfaceComposerAIDL::createConnection(sp<gui::ISurfaceComposerCli } } -binder::Status SurfaceComposerAIDL::createDisplay(const std::string& displayName, bool isSecure, - const std::string& uniqueId, - float requestedRefreshRate, - sp<IBinder>* outDisplay) { +binder::Status SurfaceComposerAIDL::createVirtualDisplay(const std::string& displayName, + bool isSecure, const std::string& uniqueId, + float requestedRefreshRate, + sp<IBinder>* outDisplay) { status_t status = checkAccessPermission(); if (status != OK) { return binderStatusFromStatusT(status); } - String8 displayName8 = String8::format("%s", displayName.c_str()); - *outDisplay = mFlinger->createDisplay(displayName8, isSecure, uniqueId, requestedRefreshRate); + *outDisplay = + mFlinger->createVirtualDisplay(displayName, isSecure, uniqueId, requestedRefreshRate); return binder::Status::ok(); } -binder::Status SurfaceComposerAIDL::destroyDisplay(const sp<IBinder>& display) { +binder::Status SurfaceComposerAIDL::destroyVirtualDisplay(const sp<IBinder>& displayToken) { status_t status = checkAccessPermission(); if (status != OK) { return binderStatusFromStatusT(status); } - mFlinger->destroyDisplay(display); - return binder::Status::ok(); + return binder::Status::fromStatusT(mFlinger->destroyVirtualDisplay(displayToken)); } binder::Status SurfaceComposerAIDL::getPhysicalDisplayIds(std::vector<int64_t>* outDisplayIds) { @@ -10354,6 +10353,11 @@ binder::Status SurfaceComposerAIDL::getSchedulingPolicy(gui::SchedulingPolicy* o return gui::getSchedulingPolicy(outPolicy); } +binder::Status SurfaceComposerAIDL::notifyShutdown() { + TransactionTraceWriter::getInstance().invoke("systemShutdown_", /* overwrite= */ false); + return ::android::binder::Status::ok(); +} + status_t SurfaceComposerAIDL::checkAccessPermission(bool usePermissionCache) { if (!mFlinger->callingThreadHasUnscopedSurfaceFlingerAccess(usePermissionCache)) { IPCThreadState* ipc = IPCThreadState::self(); diff --git a/services/surfaceflinger/SurfaceFlinger.h b/services/surfaceflinger/SurfaceFlinger.h index edcc8d3206..8a390162d9 100644 --- a/services/surfaceflinger/SurfaceFlinger.h +++ b/services/surfaceflinger/SurfaceFlinger.h @@ -547,9 +547,10 @@ private: status_t dump(int fd, const Vector<String16>& args) override { return priorityDump(fd, args); } // ISurfaceComposer implementation: - sp<IBinder> createDisplay(const String8& displayName, bool isSecure, - const std::string& uniqueId, float requestedRefreshRate = 0.0f); - void destroyDisplay(const sp<IBinder>& displayToken); + sp<IBinder> createVirtualDisplay(const std::string& displayName, bool isSecure, + const std::string& uniqueId, + float requestedRefreshRate = 0.0f); + status_t destroyVirtualDisplay(const sp<IBinder>& displayToken); std::vector<PhysicalDisplayId> getPhysicalDisplayIds() const EXCLUDES(mStateLock) { Mutex::Autolock lock(mStateLock); return getPhysicalDisplayIdsLocked(); @@ -756,13 +757,9 @@ private: const sp<DisplayDevice>&, const scheduler::RefreshRateSelector::PolicyVariant&) EXCLUDES(mStateLock) REQUIRES(kMainThreadContext); - bool shouldApplyRefreshRateSelectorPolicy(const DisplayDevice&) const - REQUIRES(mStateLock, kMainThreadContext); - // TODO(b/241285191): Look up RefreshRateSelector on Scheduler to remove redundant parameter. status_t applyRefreshRateSelectorPolicy(PhysicalDisplayId, - const scheduler::RefreshRateSelector&, - bool force = false) + const scheduler::RefreshRateSelector&) REQUIRES(mStateLock, kMainThreadContext); void commitTransactionsLegacy() EXCLUDES(mStateLock) REQUIRES(kMainThreadContext); @@ -1570,10 +1567,10 @@ public: const sp<IBinder>& layerHandle, sp<gui::IDisplayEventConnection>* outConnection) override; binder::Status createConnection(sp<gui::ISurfaceComposerClient>* outClient) override; - binder::Status createDisplay(const std::string& displayName, bool isSecure, - const std::string& uniqueId, float requestedRefreshRate, - sp<IBinder>* outDisplay) override; - binder::Status destroyDisplay(const sp<IBinder>& display) override; + binder::Status createVirtualDisplay(const std::string& displayName, bool isSecure, + const std::string& uniqueId, float requestedRefreshRate, + sp<IBinder>* outDisplay) override; + binder::Status destroyVirtualDisplay(const sp<IBinder>& displayToken) override; binder::Status getPhysicalDisplayIds(std::vector<int64_t>* outDisplayIds) override; binder::Status getPhysicalDisplayToken(int64_t displayId, sp<IBinder>* outDisplay) override; binder::Status setPowerMode(const sp<IBinder>& display, int mode) override; @@ -1684,6 +1681,7 @@ public: binder::Status getStalledTransactionInfo( int pid, std::optional<gui::StalledTransactionInfo>* outInfo) override; binder::Status getSchedulingPolicy(gui::SchedulingPolicy* outPolicy) override; + binder::Status notifyShutdown() override; private: static const constexpr bool kUsePermissionCache = true; diff --git a/services/surfaceflinger/Tracing/TransactionProtoParser.cpp b/services/surfaceflinger/Tracing/TransactionProtoParser.cpp index b3e9fab261..fc496b2982 100644 --- a/services/surfaceflinger/Tracing/TransactionProtoParser.cpp +++ b/services/surfaceflinger/Tracing/TransactionProtoParser.cpp @@ -247,7 +247,8 @@ perfetto::protos::LayerState TransactionProtoParser::toProto( proto.set_auto_refresh(layer.autoRefresh); } if (layer.what & layer_state_t::eTrustedOverlayChanged) { - proto.set_is_trusted_overlay(layer.isTrustedOverlay); + proto.set_is_trusted_overlay(layer.trustedOverlay == gui::TrustedOverlay::ENABLED); + // TODO(b/339701674) update protos } if (layer.what & layer_state_t::eBufferCropChanged) { LayerProtoHelper::writeToProto(layer.bufferCrop, proto.mutable_buffer_crop()); @@ -515,7 +516,8 @@ void TransactionProtoParser::fromProto(const perfetto::protos::LayerState& proto layer.autoRefresh = proto.auto_refresh(); } if (proto.what() & layer_state_t::eTrustedOverlayChanged) { - layer.isTrustedOverlay = proto.is_trusted_overlay(); + layer.trustedOverlay = proto.is_trusted_overlay() ? gui::TrustedOverlay::ENABLED + : gui::TrustedOverlay::UNSET; } if (proto.what() & layer_state_t::eBufferCropChanged) { LayerProtoHelper::readFromProto(proto.buffer_crop(), layer.bufferCrop); diff --git a/services/surfaceflinger/TransactionState.h b/services/surfaceflinger/TransactionState.h index 89a8f9238b..e5d648194f 100644 --- a/services/surfaceflinger/TransactionState.h +++ b/services/surfaceflinger/TransactionState.h @@ -23,6 +23,7 @@ #include "FrontEnd/LayerCreationArgs.h" #include "renderengine/ExternalTexture.h" +#include <common/FlagManager.h> #include <gui/LayerState.h> #include <system/window.h> @@ -108,9 +109,22 @@ struct TransactionState { for (const auto& state : states) { const bool frameRateChanged = state.state.what & layer_state_t::eFrameRateChanged; - if (!frameRateChanged || - state.state.frameRateCompatibility != ANATIVEWINDOW_FRAME_RATE_NO_VOTE) { - return true; + if (FlagManager::getInstance().vrr_bugfix_24q4()) { + const bool frameRateIsNoVote = frameRateChanged && + state.state.frameRateCompatibility == ANATIVEWINDOW_FRAME_RATE_NO_VOTE; + const bool frameRateCategoryChanged = + state.state.what & layer_state_t::eFrameRateCategoryChanged; + const bool frameRateCategoryIsNoPreference = frameRateCategoryChanged && + state.state.frameRateCategory == + ANATIVEWINDOW_FRAME_RATE_CATEGORY_NO_PREFERENCE; + if (!frameRateIsNoVote && !frameRateCategoryIsNoPreference) { + return true; + } + } else { + if (!frameRateChanged || + state.state.frameRateCompatibility != ANATIVEWINDOW_FRAME_RATE_NO_VOTE) { + return true; + } } } diff --git a/services/surfaceflinger/common/FlagManager.cpp b/services/surfaceflinger/common/FlagManager.cpp index 0feacacd80..57b170fba3 100644 --- a/services/surfaceflinger/common/FlagManager.cpp +++ b/services/surfaceflinger/common/FlagManager.cpp @@ -134,6 +134,7 @@ void FlagManager::dump(std::string& result) const { DUMP_READ_ONLY_FLAG(screenshot_fence_preservation); DUMP_READ_ONLY_FLAG(vulkan_renderengine); DUMP_READ_ONLY_FLAG(renderable_buffer_usage); + DUMP_READ_ONLY_FLAG(vrr_bugfix_24q4); DUMP_READ_ONLY_FLAG(restore_blur_step); DUMP_READ_ONLY_FLAG(dont_skip_on_early_ro); DUMP_READ_ONLY_FLAG(protected_if_client); @@ -146,6 +147,7 @@ void FlagManager::dump(std::string& result) const { DUMP_READ_ONLY_FLAG(detached_mirror); DUMP_READ_ONLY_FLAG(commit_not_composited); DUMP_READ_ONLY_FLAG(local_tonemap_screenshots); + DUMP_READ_ONLY_FLAG(override_trusted_overlay); #undef DUMP_READ_ONLY_FLAG #undef DUMP_SERVER_FLAG @@ -234,6 +236,7 @@ FLAG_MANAGER_READ_ONLY_FLAG(renderable_buffer_usage, "") FLAG_MANAGER_READ_ONLY_FLAG(restore_blur_step, "debug.renderengine.restore_blur_step") FLAG_MANAGER_READ_ONLY_FLAG(dont_skip_on_early_ro, "") FLAG_MANAGER_READ_ONLY_FLAG(protected_if_client, "") +FLAG_MANAGER_READ_ONLY_FLAG(vrr_bugfix_24q4, ""); FLAG_MANAGER_READ_ONLY_FLAG(ce_fence_promise, ""); FLAG_MANAGER_READ_ONLY_FLAG(graphite_renderengine, "debug.renderengine.graphite") FLAG_MANAGER_READ_ONLY_FLAG(latch_unsignaled_with_auto_refresh_changed, ""); @@ -242,6 +245,7 @@ FLAG_MANAGER_READ_ONLY_FLAG(allow_n_vsyncs_in_targeter, ""); FLAG_MANAGER_READ_ONLY_FLAG(detached_mirror, ""); FLAG_MANAGER_READ_ONLY_FLAG(commit_not_composited, ""); FLAG_MANAGER_READ_ONLY_FLAG(local_tonemap_screenshots, "debug.sf.local_tonemap_screenshots"); +FLAG_MANAGER_READ_ONLY_FLAG(override_trusted_overlay, ""); /// Trunk stable server flags /// FLAG_MANAGER_SERVER_FLAG(refresh_rate_overlay_on_external_display, "") diff --git a/services/surfaceflinger/common/include/common/FlagManager.h b/services/surfaceflinger/common/include/common/FlagManager.h index 5850fe1b02..9517ff7efb 100644 --- a/services/surfaceflinger/common/include/common/FlagManager.h +++ b/services/surfaceflinger/common/include/common/FlagManager.h @@ -72,6 +72,7 @@ public: bool enable_layer_command_batching() const; bool screenshot_fence_preservation() const; bool vulkan_renderengine() const; + bool vrr_bugfix_24q4() const; bool renderable_buffer_usage() const; bool restore_blur_step() const; bool dont_skip_on_early_ro() const; @@ -85,6 +86,7 @@ public: bool detached_mirror() const; bool commit_not_composited() const; bool local_tonemap_screenshots() const; + bool override_trusted_overlay() const; protected: // overridden for unit tests diff --git a/services/surfaceflinger/surfaceflinger_flags_new.aconfig b/services/surfaceflinger/surfaceflinger_flags_new.aconfig index 04f1d5080a..02d8819785 100644 --- a/services/surfaceflinger/surfaceflinger_flags_new.aconfig +++ b/services/surfaceflinger/surfaceflinger_flags_new.aconfig @@ -52,7 +52,7 @@ flag { metadata { purpose: PURPOSE_BUGFIX } -} # deprecate_vsync_sf +} # detached_mirror flag { name: "frame_rate_category_mrr" @@ -84,4 +84,26 @@ flag { is_fixed_read_only: true } # local_tonemap_screenshots + flag { + name: "override_trusted_overlay" + namespace: "window_surfaces" + description: "Allow child to disable trusted overlay set by a parent layer" + bug: "339701674" + is_fixed_read_only: true + metadata { + purpose: PURPOSE_BUGFIX + } +} # override_trusted_overlay + +flag { + name: "vrr_bugfix_24q4" + namespace: "core_graphics" + description: "bug fixes for VRR" + bug: "331513837" + is_fixed_read_only: true + metadata { + purpose: PURPOSE_BUGFIX + } +} # vrr_bugfix_24q4 + # IMPORTANT - please keep alphabetize to reduce merge conflicts diff --git a/services/surfaceflinger/tests/Credentials_test.cpp b/services/surfaceflinger/tests/Credentials_test.cpp index c1ef48ec97..ebe11fb0f3 100644 --- a/services/surfaceflinger/tests/Credentials_test.cpp +++ b/services/surfaceflinger/tests/Credentials_test.cpp @@ -39,8 +39,8 @@ using gui::aidl_utils::statusTFromBinderStatus; using ui::ColorMode; namespace { -const String8 DISPLAY_NAME("Credentials Display Test"); -const String8 SURFACE_NAME("Test Surface Name"); +const std::string kDisplayName("Credentials Display Test"); +const String8 kSurfaceName("Test Surface Name"); } // namespace /** @@ -99,7 +99,7 @@ protected: ASSERT_EQ(NO_ERROR, SurfaceComposerClient::getActiveDisplayMode(mDisplay, &mode)); // Background surface - mBGSurfaceControl = mComposerClient->createSurface(SURFACE_NAME, mode.resolution.getWidth(), + mBGSurfaceControl = mComposerClient->createSurface(kSurfaceName, mode.resolution.getWidth(), mode.resolution.getHeight(), PIXEL_FORMAT_RGBA_8888, 0); ASSERT_TRUE(mBGSurfaceControl != nullptr); @@ -232,7 +232,7 @@ TEST_F(CredentialsTest, SetActiveColorModeTest) { TEST_F(CredentialsTest, CreateDisplayTest) { // Only graphics and system processes can create a secure display. std::function<bool()> condition = [=]() { - sp<IBinder> testDisplay = SurfaceComposerClient::createDisplay(DISPLAY_NAME, true); + sp<IBinder> testDisplay = SurfaceComposerClient::createVirtualDisplay(kDisplayName, true); return testDisplay.get() != nullptr; }; @@ -267,7 +267,7 @@ TEST_F(CredentialsTest, CreateDisplayTest) { } condition = [=]() { - sp<IBinder> testDisplay = SurfaceComposerClient::createDisplay(DISPLAY_NAME, false); + sp<IBinder> testDisplay = SurfaceComposerClient::createVirtualDisplay(kDisplayName, false); return testDisplay.get() != nullptr; }; ASSERT_NO_FATAL_FAILURE(checkWithPrivileges(condition, true, false)); diff --git a/services/surfaceflinger/tests/MultiDisplayLayerBounds_test.cpp b/services/surfaceflinger/tests/MultiDisplayLayerBounds_test.cpp index 7fce7e9af9..56cf13d7fe 100644 --- a/services/surfaceflinger/tests/MultiDisplayLayerBounds_test.cpp +++ b/services/surfaceflinger/tests/MultiDisplayLayerBounds_test.cpp @@ -53,14 +53,15 @@ protected: } virtual void TearDown() { - SurfaceComposerClient::destroyDisplay(mVirtualDisplay); + EXPECT_EQ(NO_ERROR, SurfaceComposerClient::destroyVirtualDisplay(mVirtualDisplay)); LayerTransactionTest::TearDown(); mColorLayer = 0; } void createDisplay(const ui::Size& layerStackSize, ui::LayerStack layerStack) { + static const std::string kDisplayName("VirtualDisplay"); mVirtualDisplay = - SurfaceComposerClient::createDisplay(String8("VirtualDisplay"), false /*secure*/); + SurfaceComposerClient::createVirtualDisplay(kDisplayName, false /*isSecure*/); asTransaction([&](Transaction& t) { t.setDisplaySurface(mVirtualDisplay, mProducer); t.setDisplayLayerStack(mVirtualDisplay, layerStack); diff --git a/services/surfaceflinger/tests/TransactionTestHarnesses.h b/services/surfaceflinger/tests/TransactionTestHarnesses.h index 87e6d3ee9f..af3cb9aec1 100644 --- a/services/surfaceflinger/tests/TransactionTestHarnesses.h +++ b/services/surfaceflinger/tests/TransactionTestHarnesses.h @@ -66,8 +66,9 @@ public: sp<BufferListener> listener = sp<BufferListener>::make(this); itemConsumer->setFrameAvailableListener(listener); - vDisplay = SurfaceComposerClient::createDisplay(String8("VirtualDisplay"), - false /*secure*/); + static const std::string kDisplayName("VirtualDisplay"); + vDisplay = SurfaceComposerClient::createVirtualDisplay(kDisplayName, + false /*isSecure*/); constexpr ui::LayerStack layerStack{ 848472}; // ASCII for TTH (TransactionTestHarnesses) @@ -107,7 +108,7 @@ public: t.setLayerStack(mirrorSc, ui::INVALID_LAYER_STACK); t.apply(true); } - SurfaceComposerClient::destroyDisplay(vDisplay); + SurfaceComposerClient::destroyVirtualDisplay(vDisplay); return sc; } } diff --git a/services/surfaceflinger/tests/VirtualDisplay_test.cpp b/services/surfaceflinger/tests/VirtualDisplay_test.cpp index f31f582ffd..cd66dd20bb 100644 --- a/services/surfaceflinger/tests/VirtualDisplay_test.cpp +++ b/services/surfaceflinger/tests/VirtualDisplay_test.cpp @@ -41,14 +41,15 @@ protected: }; TEST_F(VirtualDisplayTest, VirtualDisplayDestroyedSurfaceReuse) { + static const std::string kDisplayName("VirtualDisplay"); sp<IBinder> virtualDisplay = - SurfaceComposerClient::createDisplay(String8("VirtualDisplay"), false /*secure*/); + SurfaceComposerClient::createVirtualDisplay(kDisplayName, false /*isSecure*/); SurfaceComposerClient::Transaction t; t.setDisplaySurface(virtualDisplay, mProducer); t.apply(true); - SurfaceComposerClient::destroyDisplay(virtualDisplay); + EXPECT_EQ(NO_ERROR, SurfaceComposerClient::destroyVirtualDisplay(virtualDisplay)); virtualDisplay.clear(); // Sync here to ensure the display was completely destroyed in SF t.apply(true); diff --git a/services/surfaceflinger/tests/unittests/FrameTimelineTest.cpp b/services/surfaceflinger/tests/unittests/FrameTimelineTest.cpp index 9fd687c6e7..dac9265b71 100644 --- a/services/surfaceflinger/tests/unittests/FrameTimelineTest.cpp +++ b/services/surfaceflinger/tests/unittests/FrameTimelineTest.cpp @@ -1132,6 +1132,72 @@ void validateTraceEvent(const ProtoFrameEnd& received, const ProtoFrameEnd& sour EXPECT_EQ(received.cookie(), source.cookie()); } +TEST_F(FrameTimelineTest, traceDisplayFrameNoSkipped) { + // setup 2 display frames + // DF 1: [22, 30] -> [0, 11] + // DF 2: [82, 90] -> SF [5, 16] + auto tracingSession = getTracingSessionForTest(); + tracingSession->StartBlocking(); + int64_t sfToken1 = mTokenManager->generateTokenForPredictions({22, 30, 40}); + int64_t sfToken2 = mTokenManager->generateTokenForPredictions({82, 90, 100}); + int64_t surfaceFrameToken1 = mTokenManager->generateTokenForPredictions({0, 11, 25}); + int64_t surfaceFrameToken2 = mTokenManager->generateTokenForPredictions({5, 16, 30}); + auto presentFence1 = fenceFactory.createFenceTimeForTest(Fence::NO_FENCE); + + int64_t traceCookie = snoopCurrentTraceCookie(); + + // set up 1st display frame + FrameTimelineInfo ftInfo1; + ftInfo1.vsyncId = surfaceFrameToken1; + ftInfo1.inputEventId = sInputEventId; + auto surfaceFrame1 = + mFrameTimeline->createSurfaceFrameForToken(ftInfo1, sPidOne, sUidOne, sLayerIdOne, + sLayerNameOne, sLayerNameOne, + /*isBuffer*/ true, sGameMode); + surfaceFrame1->setAcquireFenceTime(11); + mFrameTimeline->setSfWakeUp(sfToken1, 22, RR_11, RR_30); + surfaceFrame1->setPresentState(SurfaceFrame::PresentState::Presented); + mFrameTimeline->addSurfaceFrame(surfaceFrame1); + mFrameTimeline->setSfPresent(30, presentFence1); + presentFence1->signalForTest(40); + + // Trigger a flush by finalizing the next DisplayFrame + auto presentFence2 = fenceFactory.createFenceTimeForTest(Fence::NO_FENCE); + FrameTimelineInfo ftInfo2; + ftInfo2.vsyncId = surfaceFrameToken2; + ftInfo2.inputEventId = sInputEventId; + auto surfaceFrame2 = + mFrameTimeline->createSurfaceFrameForToken(ftInfo2, sPidOne, sUidOne, sLayerIdOne, + sLayerNameOne, sLayerNameOne, + /*isBuffer*/ true, sGameMode); + + // set up 2nd display frame + surfaceFrame2->setAcquireFenceTime(16); + mFrameTimeline->setSfWakeUp(sfToken2, 82, RR_11, RR_30); + surfaceFrame2->setPresentState(SurfaceFrame::PresentState::Presented); + mFrameTimeline->addSurfaceFrame(surfaceFrame2); + mFrameTimeline->setSfPresent(90, presentFence2); + presentFence2->signalForTest(100); + + // the token of skipped Display Frame + auto protoSkippedActualDisplayFrameStart = + createProtoActualDisplayFrameStart(traceCookie + 9, 0, kSurfaceFlingerPid, + FrameTimelineEvent::PRESENT_DROPPED, true, false, + FrameTimelineEvent::JANK_DROPPED, + FrameTimelineEvent::SEVERITY_NONE, + FrameTimelineEvent::PREDICTION_VALID); + auto protoSkippedActualDisplayFrameEnd = createProtoFrameEnd(traceCookie + 9); + + // Trigger a flush by finalizing the next DisplayFrame + addEmptyDisplayFrame(); + flushTrace(); + tracingSession->StopBlocking(); + + auto packets = readFrameTimelinePacketsBlocking(tracingSession.get()); + // 8 Valid Display Frames + 8 Valid Surface Frames + no Skipped Display Frames + EXPECT_EQ(packets.size(), 16u); +} + TEST_F(FrameTimelineTest, traceDisplayFrameSkipped) { SET_FLAG_FOR_TEST(com::android::graphics::surfaceflinger::flags::add_sf_skipped_frames_to_trace, true); diff --git a/services/surfaceflinger/tests/unittests/LayerHierarchyTest.h b/services/surfaceflinger/tests/unittests/LayerHierarchyTest.h index fd15eef54b..8b3303c809 100644 --- a/services/surfaceflinger/tests/unittests/LayerHierarchyTest.h +++ b/services/surfaceflinger/tests/unittests/LayerHierarchyTest.h @@ -494,14 +494,14 @@ protected: mLifecycleManager.applyTransactions(transactions); } - void setTrustedOverlay(uint32_t id, bool isTrustedOverlay) { + void setTrustedOverlay(uint32_t id, gui::TrustedOverlay trustedOverlay) { std::vector<TransactionState> transactions; transactions.emplace_back(); transactions.back().states.push_back({}); transactions.back().states.front().state.what = layer_state_t::eTrustedOverlayChanged; transactions.back().states.front().layerId = id; - transactions.back().states.front().state.isTrustedOverlay = isTrustedOverlay; + transactions.back().states.front().state.trustedOverlay = trustedOverlay; mLifecycleManager.applyTransactions(transactions); } diff --git a/services/surfaceflinger/tests/unittests/LayerLifecycleManagerTest.cpp b/services/surfaceflinger/tests/unittests/LayerLifecycleManagerTest.cpp index 867ff55632..97bd79fcee 100644 --- a/services/surfaceflinger/tests/unittests/LayerLifecycleManagerTest.cpp +++ b/services/surfaceflinger/tests/unittests/LayerLifecycleManagerTest.cpp @@ -461,8 +461,8 @@ TEST_F(LayerLifecycleManagerTest, layerOpacityChangesSetsVisibilityChangeFlag) { HAL_PIXEL_FORMAT_RGBA_8888, GRALLOC_USAGE_PROTECTED /*usage*/)); EXPECT_EQ(mLifecycleManager.getGlobalChanges().get(), - ftl::Flags<RequestedLayerState::Changes>(RequestedLayerState::Changes::Buffer | - RequestedLayerState::Changes::Content) + ftl::Flags<RequestedLayerState::Changes>( + RequestedLayerState::Changes::Buffer | RequestedLayerState::Changes::Content) .get()); mLifecycleManager.commitChanges(); @@ -493,10 +493,10 @@ TEST_F(LayerLifecycleManagerTest, bufferFormatChangesSetsVisibilityChangeFlag) { HAL_PIXEL_FORMAT_RGB_888, GRALLOC_USAGE_PROTECTED /*usage*/)); EXPECT_EQ(mLifecycleManager.getGlobalChanges().get(), - ftl::Flags<RequestedLayerState::Changes>(RequestedLayerState::Changes::Buffer | - RequestedLayerState::Changes::Content | - RequestedLayerState::Changes::VisibleRegion | - RequestedLayerState::Changes::Visibility) + ftl::Flags<RequestedLayerState::Changes>( + RequestedLayerState::Changes::Buffer | RequestedLayerState::Changes::Content | + RequestedLayerState::Changes::VisibleRegion | + RequestedLayerState::Changes::Visibility) .get()); mLifecycleManager.commitChanges(); } @@ -580,8 +580,8 @@ TEST_F(LayerLifecycleManagerTest, layerSecureChangesSetsVisibilityChangeFlag) { HAL_PIXEL_FORMAT_RGBA_8888, GRALLOC_USAGE_SW_READ_NEVER /*usage*/)); EXPECT_EQ(mLifecycleManager.getGlobalChanges().get(), - ftl::Flags<RequestedLayerState::Changes>(RequestedLayerState::Changes::Buffer | - RequestedLayerState::Changes::Content) + ftl::Flags<RequestedLayerState::Changes>( + RequestedLayerState::Changes::Buffer | RequestedLayerState::Changes::Content) .get()); mLifecycleManager.commitChanges(); @@ -592,4 +592,41 @@ TEST_F(LayerLifecycleManagerTest, layerSecureChangesSetsVisibilityChangeFlag) { mLifecycleManager.commitChanges(); } +TEST_F(LayerLifecycleManagerTest, isSimpleBufferUpdate) { + auto layer = rootLayer(1); + + // no buffer changes + EXPECT_FALSE(layer->isSimpleBufferUpdate({})); + + { + layer_state_t state; + state.what = layer_state_t::eBufferChanged; + EXPECT_TRUE(layer->isSimpleBufferUpdate(state)); + } + + { + layer_state_t state; + state.what = layer_state_t::eReparent | layer_state_t::eBufferChanged; + EXPECT_FALSE(layer->isSimpleBufferUpdate(state)); + } + + { + layer_state_t state; + state.what = layer_state_t::ePositionChanged | layer_state_t::eBufferChanged; + state.x = 9; + state.y = 10; + EXPECT_FALSE(layer->isSimpleBufferUpdate(state)); + } + + { + layer->x = 9; + layer->y = 10; + layer_state_t state; + state.what = layer_state_t::ePositionChanged | layer_state_t::eBufferChanged; + state.x = 9; + state.y = 10; + EXPECT_TRUE(layer->isSimpleBufferUpdate(state)); + } +} + } // namespace android::surfaceflinger::frontend diff --git a/services/surfaceflinger/tests/unittests/LayerSnapshotTest.cpp b/services/surfaceflinger/tests/unittests/LayerSnapshotTest.cpp index 5ff6417482..bd0accd28e 100644 --- a/services/surfaceflinger/tests/unittests/LayerSnapshotTest.cpp +++ b/services/surfaceflinger/tests/unittests/LayerSnapshotTest.cpp @@ -254,7 +254,8 @@ TEST_F(LayerSnapshotTest, UpdateClearsPreviousChangeStates) { TEST_F(LayerSnapshotTest, FastPathClearsPreviousChangeStates) { setColor(11, {1._hf, 0._hf, 0._hf}); UPDATE_AND_VERIFY(mSnapshotBuilder, STARTING_ZORDER); - EXPECT_EQ(getSnapshot(11)->changes, RequestedLayerState::Changes::Content); + EXPECT_EQ(getSnapshot(11)->changes, + RequestedLayerState::Changes::Content); EXPECT_EQ(getSnapshot(11)->clientChanges, layer_state_t::eColorChanged); EXPECT_EQ(getSnapshot(1)->changes.get(), 0u); UPDATE_AND_VERIFY(mSnapshotBuilder, STARTING_ZORDER); @@ -264,7 +265,8 @@ TEST_F(LayerSnapshotTest, FastPathClearsPreviousChangeStates) { TEST_F(LayerSnapshotTest, FastPathSetsChangeFlagToContent) { setColor(1, {1._hf, 0._hf, 0._hf}); UPDATE_AND_VERIFY(mSnapshotBuilder, STARTING_ZORDER); - EXPECT_EQ(getSnapshot(1)->changes, RequestedLayerState::Changes::Content); + EXPECT_EQ(getSnapshot(1)->changes, + RequestedLayerState::Changes::Content); EXPECT_EQ(getSnapshot(1)->clientChanges, layer_state_t::eColorChanged); } @@ -278,57 +280,13 @@ TEST_F(LayerSnapshotTest, GameMode) { transactions.back().states.front().layerId = 1; transactions.back().states.front().state.layerId = static_cast<int32_t>(1); mLifecycleManager.applyTransactions(transactions); - EXPECT_EQ(mLifecycleManager.getGlobalChanges(), - RequestedLayerState::Changes::GameMode | RequestedLayerState::Changes::Metadata); + EXPECT_EQ(mLifecycleManager.getGlobalChanges(), RequestedLayerState::Changes::GameMode); UPDATE_AND_VERIFY(mSnapshotBuilder, STARTING_ZORDER); EXPECT_EQ(getSnapshot(1)->clientChanges, layer_state_t::eMetadataChanged); EXPECT_EQ(static_cast<int32_t>(getSnapshot(1)->gameMode), 42); EXPECT_EQ(static_cast<int32_t>(getSnapshot(11)->gameMode), 42); } -TEST_F(LayerSnapshotTest, UpdateMetadata) { - std::vector<TransactionState> transactions; - transactions.emplace_back(); - transactions.back().states.push_back({}); - transactions.back().states.front().state.what = layer_state_t::eMetadataChanged; - // This test focuses on metadata used by ARC++ to ensure LayerMetadata is updated correctly, - // and not using stale data. - transactions.back().states.front().state.metadata = LayerMetadata(); - transactions.back().states.front().state.metadata.setInt32(METADATA_OWNER_UID, 123); - transactions.back().states.front().state.metadata.setInt32(METADATA_WINDOW_TYPE, 234); - transactions.back().states.front().state.metadata.setInt32(METADATA_TASK_ID, 345); - transactions.back().states.front().state.metadata.setInt32(METADATA_MOUSE_CURSOR, 456); - transactions.back().states.front().state.metadata.setInt32(METADATA_ACCESSIBILITY_ID, 567); - transactions.back().states.front().state.metadata.setInt32(METADATA_OWNER_PID, 678); - transactions.back().states.front().state.metadata.setInt32(METADATA_CALLING_UID, 789); - - transactions.back().states.front().layerId = 1; - transactions.back().states.front().state.layerId = static_cast<int32_t>(1); - - mLifecycleManager.applyTransactions(transactions); - EXPECT_EQ(mLifecycleManager.getGlobalChanges(), RequestedLayerState::Changes::Metadata); - - // Setting includeMetadata=true to ensure metadata update is applied to LayerSnapshot - LayerSnapshotBuilder::Args args{.root = mHierarchyBuilder.getHierarchy(), - .layerLifecycleManager = mLifecycleManager, - .includeMetadata = true, - .displays = mFrontEndDisplayInfos, - .globalShadowSettings = globalShadowSettings, - .supportsBlur = true, - .supportedLayerGenericMetadata = {}, - .genericLayerMetadataKeyMap = {}}; - update(mSnapshotBuilder, args); - - EXPECT_EQ(getSnapshot(1)->clientChanges, layer_state_t::eMetadataChanged); - EXPECT_EQ(getSnapshot(1)->layerMetadata.getInt32(METADATA_OWNER_UID, -1), 123); - EXPECT_EQ(getSnapshot(1)->layerMetadata.getInt32(METADATA_WINDOW_TYPE, -1), 234); - EXPECT_EQ(getSnapshot(1)->layerMetadata.getInt32(METADATA_TASK_ID, -1), 345); - EXPECT_EQ(getSnapshot(1)->layerMetadata.getInt32(METADATA_MOUSE_CURSOR, -1), 456); - EXPECT_EQ(getSnapshot(1)->layerMetadata.getInt32(METADATA_ACCESSIBILITY_ID, -1), 567); - EXPECT_EQ(getSnapshot(1)->layerMetadata.getInt32(METADATA_OWNER_PID, -1), 678); - EXPECT_EQ(getSnapshot(1)->layerMetadata.getInt32(METADATA_CALLING_UID, -1), 789); -} - TEST_F(LayerSnapshotTest, NoLayerVoteForParentWithChildVotes) { // ROOT // ├── 1 @@ -1194,7 +1152,7 @@ TEST_F(LayerSnapshotTest, setShadowRadius) { TEST_F(LayerSnapshotTest, setTrustedOverlayForNonVisibleInput) { hideLayer(1); - setTrustedOverlay(1, true); + setTrustedOverlay(1, gui::TrustedOverlay::ENABLED); Region touch{Rect{0, 0, 1000, 1000}}; setTouchableRegion(1, touch); @@ -1375,4 +1333,85 @@ TEST_F(LayerSnapshotTest, mirroredHierarchyIgnoresLocalTransform) { EXPECT_EQ(getSnapshot({.id = 111})->geomLayerTransform.ty(), 220); } +TEST_F(LayerSnapshotTest, overrideParentTrustedOverlayState) { + SET_FLAG_FOR_TEST(flags::override_trusted_overlay, true); + hideLayer(1); + setTrustedOverlay(1, gui::TrustedOverlay::ENABLED); + + Region touch{Rect{0, 0, 1000, 1000}}; + setTouchableRegion(1, touch); + setTouchableRegion(11, touch); + setTouchableRegion(111, touch); + + UPDATE_AND_VERIFY(mSnapshotBuilder, {2}); + EXPECT_TRUE(getSnapshot(1)->inputInfo.inputConfig.test( + gui::WindowInfo::InputConfig::TRUSTED_OVERLAY)); + EXPECT_TRUE(getSnapshot(11)->inputInfo.inputConfig.test( + gui::WindowInfo::InputConfig::TRUSTED_OVERLAY)); + EXPECT_TRUE(getSnapshot(111)->inputInfo.inputConfig.test( + gui::WindowInfo::InputConfig::TRUSTED_OVERLAY)); + + // disable trusted overlay and override parent state + setTrustedOverlay(11, gui::TrustedOverlay::DISABLED); + UPDATE_AND_VERIFY(mSnapshotBuilder, {2}); + EXPECT_TRUE(getSnapshot(1)->inputInfo.inputConfig.test( + gui::WindowInfo::InputConfig::TRUSTED_OVERLAY)); + EXPECT_FALSE(getSnapshot(11)->inputInfo.inputConfig.test( + gui::WindowInfo::InputConfig::TRUSTED_OVERLAY)); + EXPECT_FALSE(getSnapshot(111)->inputInfo.inputConfig.test( + gui::WindowInfo::InputConfig::TRUSTED_OVERLAY)); + + // unset state and go back to default behavior of inheriting + // state + setTrustedOverlay(11, gui::TrustedOverlay::UNSET); + UPDATE_AND_VERIFY(mSnapshotBuilder, {2}); + EXPECT_TRUE(getSnapshot(1)->inputInfo.inputConfig.test( + gui::WindowInfo::InputConfig::TRUSTED_OVERLAY)); + EXPECT_TRUE(getSnapshot(11)->inputInfo.inputConfig.test( + gui::WindowInfo::InputConfig::TRUSTED_OVERLAY)); + EXPECT_TRUE(getSnapshot(111)->inputInfo.inputConfig.test( + gui::WindowInfo::InputConfig::TRUSTED_OVERLAY)); +} + +TEST_F(LayerSnapshotTest, doNotOverrideParentTrustedOverlayState) { + SET_FLAG_FOR_TEST(flags::override_trusted_overlay, false); + hideLayer(1); + setTrustedOverlay(1, gui::TrustedOverlay::ENABLED); + + Region touch{Rect{0, 0, 1000, 1000}}; + setTouchableRegion(1, touch); + setTouchableRegion(11, touch); + setTouchableRegion(111, touch); + + UPDATE_AND_VERIFY(mSnapshotBuilder, {2}); + EXPECT_TRUE(getSnapshot(1)->inputInfo.inputConfig.test( + gui::WindowInfo::InputConfig::TRUSTED_OVERLAY)); + EXPECT_TRUE(getSnapshot(11)->inputInfo.inputConfig.test( + gui::WindowInfo::InputConfig::TRUSTED_OVERLAY)); + EXPECT_TRUE(getSnapshot(111)->inputInfo.inputConfig.test( + gui::WindowInfo::InputConfig::TRUSTED_OVERLAY)); + + // disable trusted overlay but flag is disabled so this behaves + // as UNSET + setTrustedOverlay(11, gui::TrustedOverlay::DISABLED); + UPDATE_AND_VERIFY(mSnapshotBuilder, {2}); + EXPECT_TRUE(getSnapshot(1)->inputInfo.inputConfig.test( + gui::WindowInfo::InputConfig::TRUSTED_OVERLAY)); + EXPECT_TRUE(getSnapshot(11)->inputInfo.inputConfig.test( + gui::WindowInfo::InputConfig::TRUSTED_OVERLAY)); + EXPECT_TRUE(getSnapshot(111)->inputInfo.inputConfig.test( + gui::WindowInfo::InputConfig::TRUSTED_OVERLAY)); + + // unset state and go back to default behavior of inheriting + // state + setTrustedOverlay(11, gui::TrustedOverlay::UNSET); + UPDATE_AND_VERIFY(mSnapshotBuilder, {2}); + EXPECT_TRUE(getSnapshot(1)->inputInfo.inputConfig.test( + gui::WindowInfo::InputConfig::TRUSTED_OVERLAY)); + EXPECT_TRUE(getSnapshot(11)->inputInfo.inputConfig.test( + gui::WindowInfo::InputConfig::TRUSTED_OVERLAY)); + EXPECT_TRUE(getSnapshot(111)->inputInfo.inputConfig.test( + gui::WindowInfo::InputConfig::TRUSTED_OVERLAY)); +} + } // namespace android::surfaceflinger::frontend diff --git a/services/surfaceflinger/tests/unittests/SchedulerTest.cpp b/services/surfaceflinger/tests/unittests/SchedulerTest.cpp index 7479a4f890..4fb06907d0 100644 --- a/services/surfaceflinger/tests/unittests/SchedulerTest.cpp +++ b/services/surfaceflinger/tests/unittests/SchedulerTest.cpp @@ -350,6 +350,9 @@ TEST_F(SchedulerTest, chooseDisplayModesMultipleDisplays) { std::make_shared<RefreshRateSelector>(kDisplay2Modes, kDisplay2Mode60->getId())); + mScheduler->setDisplayPowerMode(kDisplayId1, hal::PowerMode::ON); + mScheduler->setDisplayPowerMode(kDisplayId2, hal::PowerMode::ON); + using DisplayModeChoice = TestableScheduler::DisplayModeChoice; TestableScheduler::DisplayModeChoiceMap expectedChoices; @@ -412,6 +415,7 @@ TEST_F(SchedulerTest, chooseDisplayModesMultipleDisplays) { ->registerDisplay(kDisplayId3, std::make_shared<RefreshRateSelector>(kDisplay3Modes, kDisplay3Mode60->getId())); + mScheduler->setDisplayPowerMode(kDisplayId3, hal::PowerMode::ON); const GlobalSignals globalSignals = {.touch = true}; mScheduler->replaceTouchTimer(10); diff --git a/services/surfaceflinger/tests/unittests/SurfaceFlinger_ColorMatrixTest.cpp b/services/surfaceflinger/tests/unittests/SurfaceFlinger_ColorMatrixTest.cpp index 5852b1c45f..ff7612e064 100644 --- a/services/surfaceflinger/tests/unittests/SurfaceFlinger_ColorMatrixTest.cpp +++ b/services/surfaceflinger/tests/unittests/SurfaceFlinger_ColorMatrixTest.cpp @@ -53,7 +53,8 @@ TEST_F(ColorMatrixTest, colorMatrixChangedAfterDisplayTransaction) { mFlinger.commitAndComposite(); EXPECT_COLOR_MATRIX_CHANGED(false, false); - mFlinger.createDisplay(String8("Test Display"), false); + static const std::string kDisplayName("Test Display"); + mFlinger.createVirtualDisplay(kDisplayName, false /*isSecure=*/); mFlinger.commit(); EXPECT_COLOR_MATRIX_CHANGED(false, true); diff --git a/services/surfaceflinger/tests/unittests/SurfaceFlinger_CreateDisplayTest.cpp b/services/surfaceflinger/tests/unittests/SurfaceFlinger_CreateDisplayTest.cpp index bf5ae21f60..e5f2a9138d 100644 --- a/services/surfaceflinger/tests/unittests/SurfaceFlinger_CreateDisplayTest.cpp +++ b/services/surfaceflinger/tests/unittests/SurfaceFlinger_CreateDisplayTest.cpp @@ -27,7 +27,7 @@ namespace { class CreateDisplayTest : public DisplayTransactionTest { public: - void createDisplayWithRequestedRefreshRate(const String8& name, uint64_t displayId, + void createDisplayWithRequestedRefreshRate(const std::string& name, uint64_t displayId, float pacesetterDisplayRefreshRate, float requestedRefreshRate, float expectedAdjustedRefreshRate) { @@ -37,7 +37,7 @@ public: // -------------------------------------------------------------------- // Invocation - sp<IBinder> displayToken = mFlinger.createDisplay(name, false, requestedRefreshRate); + sp<IBinder> displayToken = mFlinger.createVirtualDisplay(name, false, requestedRefreshRate); // -------------------------------------------------------------------- // Postconditions @@ -73,7 +73,7 @@ public: }; TEST_F(CreateDisplayTest, createDisplaySetsCurrentStateForNonsecureDisplay) { - const String8 name("virtual.test"); + static const std::string name("virtual.test"); // -------------------------------------------------------------------- // Call Expectations @@ -81,7 +81,7 @@ TEST_F(CreateDisplayTest, createDisplaySetsCurrentStateForNonsecureDisplay) { // -------------------------------------------------------------------- // Invocation - sp<IBinder> displayToken = mFlinger.createDisplay(name, false); + sp<IBinder> displayToken = mFlinger.createVirtualDisplay(name, false); // -------------------------------------------------------------------- // Postconditions @@ -101,7 +101,7 @@ TEST_F(CreateDisplayTest, createDisplaySetsCurrentStateForNonsecureDisplay) { } TEST_F(CreateDisplayTest, createDisplaySetsCurrentStateForSecureDisplay) { - const String8 name("virtual.test"); + static const std::string kDisplayName("virtual.test"); // -------------------------------------------------------------------- // Call Expectations @@ -112,7 +112,7 @@ TEST_F(CreateDisplayTest, createDisplaySetsCurrentStateForSecureDisplay) { // Set the calling identity to graphics so captureDisplay with secure is allowed. IPCThreadState::self()->restoreCallingIdentity(static_cast<int64_t>(AID_GRAPHICS) << 32 | AID_GRAPHICS); - sp<IBinder> displayToken = mFlinger.createDisplay(name, true); + sp<IBinder> displayToken = mFlinger.createVirtualDisplay(kDisplayName, true); IPCThreadState::self()->restoreCallingIdentity(oldId); // -------------------------------------------------------------------- @@ -123,7 +123,7 @@ TEST_F(CreateDisplayTest, createDisplaySetsCurrentStateForSecureDisplay) { const auto& display = getCurrentDisplayState(displayToken); EXPECT_TRUE(display.isVirtual()); EXPECT_TRUE(display.isSecure); - EXPECT_EQ(name.c_str(), display.displayName); + EXPECT_EQ(kDisplayName.c_str(), display.displayName); // -------------------------------------------------------------------- // Cleanup conditions @@ -133,8 +133,8 @@ TEST_F(CreateDisplayTest, createDisplaySetsCurrentStateForSecureDisplay) { } TEST_F(CreateDisplayTest, createDisplaySetsCurrentStateForUniqueId) { - const String8 name("virtual.test"); - const std::string uniqueId = "virtual:package:id"; + static const std::string kDisplayName("virtual.test"); + static const std::string kUniqueId = "virtual:package:id"; // -------------------------------------------------------------------- // Call Expectations @@ -142,7 +142,7 @@ TEST_F(CreateDisplayTest, createDisplaySetsCurrentStateForUniqueId) { // -------------------------------------------------------------------- // Invocation - sp<IBinder> displayToken = mFlinger.createDisplay(name, false, uniqueId); + sp<IBinder> displayToken = mFlinger.createVirtualDisplay(kDisplayName, false, kUniqueId); // -------------------------------------------------------------------- // Postconditions @@ -153,7 +153,7 @@ TEST_F(CreateDisplayTest, createDisplaySetsCurrentStateForUniqueId) { EXPECT_TRUE(display.isVirtual()); EXPECT_FALSE(display.isSecure); EXPECT_EQ(display.uniqueId, "virtual:package:id"); - EXPECT_EQ(name.c_str(), display.displayName); + EXPECT_EQ(kDisplayName.c_str(), display.displayName); // -------------------------------------------------------------------- // Cleanup conditions @@ -164,78 +164,78 @@ TEST_F(CreateDisplayTest, createDisplaySetsCurrentStateForUniqueId) { // Requesting 0 tells SF not to do anything, i.e., default to refresh as physical displays TEST_F(CreateDisplayTest, createDisplayWithRequestedRefreshRate0) { - const String8 displayName("virtual.test"); - const uint64_t displayId = 123ull; - const float kPacesetterDisplayRefreshRate = 60.f; - const float kRequestedRefreshRate = 0.f; - const float kExpectedAdjustedRefreshRate = 0.f; - createDisplayWithRequestedRefreshRate(displayName, displayId, kPacesetterDisplayRefreshRate, + static const std::string kDisplayName("virtual.test"); + constexpr uint64_t kDisplayId = 123ull; + constexpr float kPacesetterDisplayRefreshRate = 60.f; + constexpr float kRequestedRefreshRate = 0.f; + constexpr float kExpectedAdjustedRefreshRate = 0.f; + createDisplayWithRequestedRefreshRate(kDisplayName, kDisplayId, kPacesetterDisplayRefreshRate, kRequestedRefreshRate, kExpectedAdjustedRefreshRate); } // Requesting negative refresh rate, will be ignored, same as requesting 0 TEST_F(CreateDisplayTest, createDisplayWithRequestedRefreshRateNegative) { - const String8 displayName("virtual.test"); - const uint64_t displayId = 123ull; - const float kPacesetterDisplayRefreshRate = 60.f; - const float kRequestedRefreshRate = -60.f; - const float kExpectedAdjustedRefreshRate = 0.f; - createDisplayWithRequestedRefreshRate(displayName, displayId, kPacesetterDisplayRefreshRate, + static const std::string kDisplayName("virtual.test"); + constexpr uint64_t kDisplayId = 123ull; + constexpr float kPacesetterDisplayRefreshRate = 60.f; + constexpr float kRequestedRefreshRate = -60.f; + constexpr float kExpectedAdjustedRefreshRate = 0.f; + createDisplayWithRequestedRefreshRate(kDisplayName, kDisplayId, kPacesetterDisplayRefreshRate, kRequestedRefreshRate, kExpectedAdjustedRefreshRate); } // Requesting a higher refresh rate than the pacesetter TEST_F(CreateDisplayTest, createDisplayWithRequestedRefreshRateHigh) { - const String8 displayName("virtual.test"); - const uint64_t displayId = 123ull; - const float kPacesetterDisplayRefreshRate = 60.f; - const float kRequestedRefreshRate = 90.f; - const float kExpectedAdjustedRefreshRate = 60.f; - createDisplayWithRequestedRefreshRate(displayName, displayId, kPacesetterDisplayRefreshRate, + static const std::string kDisplayName("virtual.test"); + constexpr uint64_t kDisplayId = 123ull; + constexpr float kPacesetterDisplayRefreshRate = 60.f; + constexpr float kRequestedRefreshRate = 90.f; + constexpr float kExpectedAdjustedRefreshRate = 60.f; + createDisplayWithRequestedRefreshRate(kDisplayName, kDisplayId, kPacesetterDisplayRefreshRate, kRequestedRefreshRate, kExpectedAdjustedRefreshRate); } // Requesting the same refresh rate as the pacesetter TEST_F(CreateDisplayTest, createDisplayWithRequestedRefreshRateSame) { - const String8 displayName("virtual.test"); - const uint64_t displayId = 123ull; - const float kPacesetterDisplayRefreshRate = 60.f; - const float kRequestedRefreshRate = 60.f; - const float kExpectedAdjustedRefreshRate = 60.f; - createDisplayWithRequestedRefreshRate(displayName, displayId, kPacesetterDisplayRefreshRate, + static const std::string kDisplayName("virtual.test"); + constexpr uint64_t kDisplayId = 123ull; + constexpr float kPacesetterDisplayRefreshRate = 60.f; + constexpr float kRequestedRefreshRate = 60.f; + constexpr float kExpectedAdjustedRefreshRate = 60.f; + createDisplayWithRequestedRefreshRate(kDisplayName, kDisplayId, kPacesetterDisplayRefreshRate, kRequestedRefreshRate, kExpectedAdjustedRefreshRate); } // Requesting a divisor (30) of the pacesetter (60) should be honored TEST_F(CreateDisplayTest, createDisplayWithRequestedRefreshRateDivisor) { - const String8 displayName("virtual.test"); - const uint64_t displayId = 123ull; - const float kPacesetterDisplayRefreshRate = 60.f; - const float kRequestedRefreshRate = 30.f; - const float kExpectedAdjustedRefreshRate = 30.f; - createDisplayWithRequestedRefreshRate(displayName, displayId, kPacesetterDisplayRefreshRate, + static const std::string kDisplayName("virtual.test"); + constexpr uint64_t kDisplayId = 123ull; + constexpr float kPacesetterDisplayRefreshRate = 60.f; + constexpr float kRequestedRefreshRate = 30.f; + constexpr float kExpectedAdjustedRefreshRate = 30.f; + createDisplayWithRequestedRefreshRate(kDisplayName, kDisplayId, kPacesetterDisplayRefreshRate, kRequestedRefreshRate, kExpectedAdjustedRefreshRate); } // Requesting a non divisor (45) of the pacesetter (120) should round up to a divisor (60) TEST_F(CreateDisplayTest, createDisplayWithRequestedRefreshRateNoneDivisor) { - const String8 displayName("virtual.test"); - const uint64_t displayId = 123ull; - const float kPacesetterDisplayRefreshRate = 120.f; - const float kRequestedRefreshRate = 45.f; - const float kExpectedAdjustedRefreshRate = 60.f; - createDisplayWithRequestedRefreshRate(displayName, displayId, kPacesetterDisplayRefreshRate, + static const std::string kDisplayName("virtual.test"); + constexpr uint64_t kDisplayId = 123ull; + constexpr float kPacesetterDisplayRefreshRate = 120.f; + constexpr float kRequestedRefreshRate = 45.f; + constexpr float kExpectedAdjustedRefreshRate = 60.f; + createDisplayWithRequestedRefreshRate(kDisplayName, kDisplayId, kPacesetterDisplayRefreshRate, kRequestedRefreshRate, kExpectedAdjustedRefreshRate); } // Requesting a non divisor (75) of the pacesetter (120) should round up to pacesetter (120) TEST_F(CreateDisplayTest, createDisplayWithRequestedRefreshRateNoneDivisorMax) { - const String8 displayName("virtual.test"); - const uint64_t displayId = 123ull; - const float kPacesetterDisplayRefreshRate = 120.f; - const float kRequestedRefreshRate = 75.f; - const float kExpectedAdjustedRefreshRate = 120.f; - createDisplayWithRequestedRefreshRate(displayName, displayId, kPacesetterDisplayRefreshRate, + static const std::string kDisplayName("virtual.test"); + constexpr uint64_t kDisplayId = 123ull; + constexpr float kPacesetterDisplayRefreshRate = 120.f; + constexpr float kRequestedRefreshRate = 75.f; + constexpr float kExpectedAdjustedRefreshRate = 120.f; + createDisplayWithRequestedRefreshRate(kDisplayName, kDisplayId, kPacesetterDisplayRefreshRate, kRequestedRefreshRate, kExpectedAdjustedRefreshRate); } diff --git a/services/surfaceflinger/tests/unittests/SurfaceFlinger_DestroyDisplayTest.cpp b/services/surfaceflinger/tests/unittests/SurfaceFlinger_DestroyDisplayTest.cpp index 93a3811172..f8ad8e1e1b 100644 --- a/services/surfaceflinger/tests/unittests/SurfaceFlinger_DestroyDisplayTest.cpp +++ b/services/surfaceflinger/tests/unittests/SurfaceFlinger_DestroyDisplayTest.cpp @@ -43,18 +43,18 @@ TEST_F(DestroyDisplayTest, destroyDisplayClearsCurrentStateForDisplay) { // -------------------------------------------------------------------- // Invocation - mFlinger.destroyDisplay(existing.token()); + EXPECT_EQ(NO_ERROR, mFlinger.destroyVirtualDisplay(existing.token())); // -------------------------------------------------------------------- // Postconditions - // The display should have been removed from the current state + // The display should have been removed from the current state. EXPECT_FALSE(hasCurrentDisplayState(existing.token())); - // Ths display should still exist in the drawing state + // Ths display should still exist in the drawing state. EXPECT_TRUE(hasDrawingDisplayState(existing.token())); - // The display transaction needed flasg should be set + // The display transaction needed flags should be set. EXPECT_TRUE(hasTransactionFlagSet(eDisplayTransactionNeeded)); } @@ -67,7 +67,7 @@ TEST_F(DestroyDisplayTest, destroyDisplayHandlesUnknownDisplay) { // -------------------------------------------------------------------- // Invocation - mFlinger.destroyDisplay(displayToken); + EXPECT_EQ(NAME_NOT_FOUND, mFlinger.destroyVirtualDisplay(displayToken)); } } // namespace diff --git a/services/surfaceflinger/tests/unittests/SurfaceFlinger_DisplayModeSwitching.cpp b/services/surfaceflinger/tests/unittests/SurfaceFlinger_DisplayModeSwitching.cpp index 15a6db626a..078b4fe15e 100644 --- a/services/surfaceflinger/tests/unittests/SurfaceFlinger_DisplayModeSwitching.cpp +++ b/services/surfaceflinger/tests/unittests/SurfaceFlinger_DisplayModeSwitching.cpp @@ -327,7 +327,9 @@ MATCHER_P2(ModeSwitchingTo, flinger, modeId, "") { return false; } - if (!flinger->scheduler()->vsyncModulator().isVsyncConfigEarly()) { + // VsyncModulator should react to mode switches on the pacesetter display. + if (arg->getPhysicalId() == flinger->scheduler()->pacesetterDisplayId() && + !flinger->scheduler()->vsyncModulator().isVsyncConfigEarly()) { *result_listener << "VsyncModulator did not shift to early phase"; return false; } @@ -368,8 +370,8 @@ TEST_F(DisplayModeSwitchingTest, innerXorOuterDisplay) { EXPECT_THAT(innerDisplay, ModeSettledTo(kModeId60)); EXPECT_THAT(outerDisplay, ModeSettledTo(kModeId120)); - // Only the inner display is powered on. - mFlinger.onActiveDisplayChanged(nullptr, *innerDisplay); + mFlinger.setPowerModeInternal(outerDisplay, hal::PowerMode::OFF); + mFlinger.setPowerModeInternal(innerDisplay, hal::PowerMode::ON); EXPECT_THAT(innerDisplay, ModeSettledTo(kModeId60)); EXPECT_THAT(outerDisplay, ModeSettledTo(kModeId120)); @@ -385,40 +387,44 @@ TEST_F(DisplayModeSwitchingTest, innerXorOuterDisplay) { 0.f, 120.f))); EXPECT_THAT(innerDisplay, ModeSwitchingTo(&mFlinger, kModeId90)); - EXPECT_THAT(outerDisplay, ModeSettledTo(kModeId120)); + EXPECT_THAT(outerDisplay, ModeSwitchingTo(&mFlinger, kModeId60)); const VsyncPeriodChangeTimeline timeline{.refreshRequired = true}; EXPECT_SET_ACTIVE_CONFIG(kInnerDisplayHwcId, kModeId90); + EXPECT_SET_ACTIVE_CONFIG(kOuterDisplayHwcId, kModeId60); mFlinger.commit(); EXPECT_THAT(innerDisplay, ModeSwitchingTo(&mFlinger, kModeId90)); - EXPECT_THAT(outerDisplay, ModeSettledTo(kModeId120)); + EXPECT_THAT(outerDisplay, ModeSwitchingTo(&mFlinger, kModeId60)); mFlinger.commit(); EXPECT_THAT(innerDisplay, ModeSettledTo(kModeId90)); - EXPECT_THAT(outerDisplay, ModeSettledTo(kModeId120)); - - innerDisplay->setPowerMode(hal::PowerMode::OFF); - outerDisplay->setPowerMode(hal::PowerMode::ON); + EXPECT_THAT(outerDisplay, ModeSettledTo(kModeId60)); - // Only the outer display is powered on. - mFlinger.onActiveDisplayChanged(innerDisplay.get(), *outerDisplay); + mFlinger.setPowerModeInternal(innerDisplay, hal::PowerMode::OFF); + mFlinger.setPowerModeInternal(outerDisplay, hal::PowerMode::ON); EXPECT_THAT(innerDisplay, ModeSettledTo(kModeId90)); - EXPECT_THAT(outerDisplay, ModeSwitchingTo(&mFlinger, kModeId60)); + EXPECT_THAT(outerDisplay, ModeSettledTo(kModeId60)); - EXPECT_SET_ACTIVE_CONFIG(kOuterDisplayHwcId, kModeId60); + EXPECT_EQ(NO_ERROR, + mFlinger.setDesiredDisplayModeSpecs(innerDisplay->getDisplayToken().promote(), + mock::createDisplayModeSpecs(kModeId60, false, + 0.f, 120.f))); + + EXPECT_THAT(innerDisplay, ModeSwitchingTo(&mFlinger, kModeId60)); + EXPECT_SET_ACTIVE_CONFIG(kInnerDisplayHwcId, kModeId60); mFlinger.commit(); - EXPECT_THAT(innerDisplay, ModeSettledTo(kModeId90)); - EXPECT_THAT(outerDisplay, ModeSwitchingTo(&mFlinger, kModeId60)); + EXPECT_THAT(innerDisplay, ModeSwitchingTo(&mFlinger, kModeId60)); + EXPECT_THAT(outerDisplay, ModeSettledTo(kModeId60)); mFlinger.commit(); - EXPECT_THAT(innerDisplay, ModeSettledTo(kModeId90)); + EXPECT_THAT(innerDisplay, ModeSettledTo(kModeId60)); EXPECT_THAT(outerDisplay, ModeSettledTo(kModeId60)); } @@ -437,10 +443,8 @@ TEST_F(DisplayModeSwitchingTest, innerAndOuterDisplay) { EXPECT_THAT(innerDisplay, ModeSettledTo(kModeId60)); EXPECT_THAT(outerDisplay, ModeSettledTo(kModeId120)); - outerDisplay->setPowerMode(hal::PowerMode::ON); - - // Both displays are powered on. - mFlinger.onActiveDisplayChanged(nullptr, *innerDisplay); + mFlinger.setPowerModeInternal(innerDisplay, hal::PowerMode::ON); + mFlinger.setPowerModeInternal(outerDisplay, hal::PowerMode::ON); EXPECT_THAT(innerDisplay, ModeSettledTo(kModeId60)); EXPECT_THAT(outerDisplay, ModeSettledTo(kModeId120)); @@ -485,7 +489,7 @@ TEST_F(DisplayModeSwitchingTest, powerOffDuringModeSet) { EXPECT_THAT(mDisplay, ModeSwitchingTo(&mFlinger, kModeId90)); // Power off the display before the mode has been set. - mDisplay->setPowerMode(hal::PowerMode::OFF); + mFlinger.setPowerModeInternal(mDisplay, hal::PowerMode::OFF); const VsyncPeriodChangeTimeline timeline{.refreshRequired = true}; EXPECT_SET_ACTIVE_CONFIG(kInnerDisplayHwcId, kModeId90); @@ -517,10 +521,8 @@ TEST_F(DisplayModeSwitchingTest, powerOffDuringConcurrentModeSet) { EXPECT_THAT(innerDisplay, ModeSettledTo(kModeId60)); EXPECT_THAT(outerDisplay, ModeSettledTo(kModeId120)); - outerDisplay->setPowerMode(hal::PowerMode::ON); - - // Both displays are powered on. - mFlinger.onActiveDisplayChanged(nullptr, *innerDisplay); + mFlinger.setPowerModeInternal(innerDisplay, hal::PowerMode::ON); + mFlinger.setPowerModeInternal(outerDisplay, hal::PowerMode::ON); EXPECT_THAT(innerDisplay, ModeSettledTo(kModeId60)); EXPECT_THAT(outerDisplay, ModeSettledTo(kModeId120)); @@ -539,40 +541,42 @@ TEST_F(DisplayModeSwitchingTest, powerOffDuringConcurrentModeSet) { EXPECT_THAT(outerDisplay, ModeSwitchingTo(&mFlinger, kModeId60)); // Power off the outer display before the mode has been set. - outerDisplay->setPowerMode(hal::PowerMode::OFF); + mFlinger.setPowerModeInternal(outerDisplay, hal::PowerMode::OFF); const VsyncPeriodChangeTimeline timeline{.refreshRequired = true}; EXPECT_SET_ACTIVE_CONFIG(kInnerDisplayHwcId, kModeId90); + EXPECT_SET_ACTIVE_CONFIG(kOuterDisplayHwcId, kModeId60); mFlinger.commit(); - // Powering off the inactive display should abort the mode set. + // Powering off the inactive display should not abort the mode set. EXPECT_THAT(innerDisplay, ModeSwitchingTo(&mFlinger, kModeId90)); - EXPECT_THAT(outerDisplay, ModeSettledTo(kModeId120)); + EXPECT_THAT(outerDisplay, ModeSwitchingTo(&mFlinger, kModeId60)); mFlinger.commit(); EXPECT_THAT(innerDisplay, ModeSettledTo(kModeId90)); - EXPECT_THAT(outerDisplay, ModeSettledTo(kModeId120)); + EXPECT_THAT(outerDisplay, ModeSettledTo(kModeId60)); - innerDisplay->setPowerMode(hal::PowerMode::OFF); - outerDisplay->setPowerMode(hal::PowerMode::ON); + mFlinger.setPowerModeInternal(innerDisplay, hal::PowerMode::OFF); + mFlinger.setPowerModeInternal(outerDisplay, hal::PowerMode::ON); - // Only the outer display is powered on. - mFlinger.onActiveDisplayChanged(innerDisplay.get(), *outerDisplay); + EXPECT_EQ(NO_ERROR, + mFlinger.setDesiredDisplayModeSpecs(outerDisplay->getDisplayToken().promote(), + mock::createDisplayModeSpecs(kModeId120, false, + 0.f, 120.f))); - EXPECT_SET_ACTIVE_CONFIG(kOuterDisplayHwcId, kModeId60); + EXPECT_SET_ACTIVE_CONFIG(kOuterDisplayHwcId, kModeId120); mFlinger.commit(); - // The mode set should resume once the display becomes active. EXPECT_THAT(innerDisplay, ModeSettledTo(kModeId90)); - EXPECT_THAT(outerDisplay, ModeSwitchingTo(&mFlinger, kModeId60)); + EXPECT_THAT(outerDisplay, ModeSwitchingTo(&mFlinger, kModeId120)); mFlinger.commit(); EXPECT_THAT(innerDisplay, ModeSettledTo(kModeId90)); - EXPECT_THAT(outerDisplay, ModeSettledTo(kModeId60)); + EXPECT_THAT(outerDisplay, ModeSettledTo(kModeId120)); } } // namespace diff --git a/services/surfaceflinger/tests/unittests/SurfaceFlinger_GetDisplayStatsTest.cpp b/services/surfaceflinger/tests/unittests/SurfaceFlinger_GetDisplayStatsTest.cpp index 4e9fba7bda..f424133655 100644 --- a/services/surfaceflinger/tests/unittests/SurfaceFlinger_GetDisplayStatsTest.cpp +++ b/services/surfaceflinger/tests/unittests/SurfaceFlinger_GetDisplayStatsTest.cpp @@ -42,8 +42,8 @@ TEST_F(SurfaceFlingerGetDisplayStatsTest, explicitToken) { } TEST_F(SurfaceFlingerGetDisplayStatsTest, invalidToken) { - const String8 displayName("fakeDisplay"); - sp<IBinder> displayToken = mFlinger.createDisplay(displayName, false); + static const std::string kDisplayName("fakeDisplay"); + sp<IBinder> displayToken = mFlinger.createVirtualDisplay(kDisplayName, false /*isSecure*/); DisplayStatInfo info; status_t status = mFlinger.getDisplayStats(displayToken, &info); EXPECT_EQ(status, NAME_NOT_FOUND); diff --git a/services/surfaceflinger/tests/unittests/TestableScheduler.h b/services/surfaceflinger/tests/unittests/TestableScheduler.h index 1e02c67d91..198a5def8c 100644 --- a/services/surfaceflinger/tests/unittests/TestableScheduler.h +++ b/services/surfaceflinger/tests/unittests/TestableScheduler.h @@ -111,6 +111,11 @@ public: Scheduler::unregisterDisplay(displayId); } + void setDisplayPowerMode(PhysicalDisplayId displayId, hal::PowerMode powerMode) { + ftl::FakeGuard guard(kMainThreadContext); + Scheduler::setDisplayPowerMode(displayId, powerMode); + } + std::optional<PhysicalDisplayId> pacesetterDisplayId() const NO_THREAD_SAFETY_ANALYSIS { return mPacesetterDisplayId; } diff --git a/services/surfaceflinger/tests/unittests/TestableSurfaceFlinger.h b/services/surfaceflinger/tests/unittests/TestableSurfaceFlinger.h index b251e2cf0c..265f804fc8 100644 --- a/services/surfaceflinger/tests/unittests/TestableSurfaceFlinger.h +++ b/services/surfaceflinger/tests/unittests/TestableSurfaceFlinger.h @@ -420,19 +420,21 @@ public: commit(kComposite); } - auto createDisplay(const String8& displayName, bool isSecure, - float requestedRefreshRate = 0.0f) { - const std::string testId = "virtual:libsurfaceflinger_unittest:TestableSurfaceFlinger"; - return mFlinger->createDisplay(displayName, isSecure, testId, requestedRefreshRate); + auto createVirtualDisplay(const std::string& displayName, bool isSecure, + float requestedRefreshRate = 0.0f) { + static const std::string kTestId = + "virtual:libsurfaceflinger_unittest:TestableSurfaceFlinger"; + return mFlinger->createVirtualDisplay(displayName, isSecure, kTestId, requestedRefreshRate); } - auto createDisplay(const String8& displayName, bool isSecure, const std::string& uniqueId, - float requestedRefreshRate = 0.0f) { - return mFlinger->createDisplay(displayName, isSecure, uniqueId, requestedRefreshRate); + auto createVirtualDisplay(const std::string& displayName, bool isSecure, + const std::string& uniqueId, float requestedRefreshRate = 0.0f) { + return mFlinger->createVirtualDisplay(displayName, isSecure, uniqueId, + requestedRefreshRate); } - auto destroyDisplay(const sp<IBinder>& displayToken) { - return mFlinger->destroyDisplay(displayToken); + auto destroyVirtualDisplay(const sp<IBinder>& displayToken) { + return mFlinger->destroyVirtualDisplay(displayToken); } auto getDisplay(const sp<IBinder>& displayToken) { diff --git a/vulkan/libvulkan/driver.cpp b/vulkan/libvulkan/driver.cpp index 7ea98f5469..3f89960e32 100644 --- a/vulkan/libvulkan/driver.cpp +++ b/vulkan/libvulkan/driver.cpp @@ -964,14 +964,20 @@ PFN_vkVoidFunction GetInstanceProcAddr(VkInstance instance, const char* pName) { PFN_vkVoidFunction GetDeviceProcAddr(VkDevice device, const char* pName) { const ProcHook* hook = GetProcHook(pName); + PFN_vkVoidFunction drv_func = GetData(device).driver.GetDeviceProcAddr(device, pName); + if (!hook) - return GetData(device).driver.GetDeviceProcAddr(device, pName); + return drv_func; if (hook->type != ProcHook::DEVICE) { ALOGE("internal vkGetDeviceProcAddr called for %s", pName); return nullptr; } + // Don't hook if we don't have a device entry function below for the core function. + if (!drv_func && (hook->extension >= ProcHook::EXTENSION_CORE_1_0)) + return nullptr; + return (GetData(device).hook_extensions[hook->extension]) ? hook->proc : nullptr; } diff --git a/vulkan/scripts/driver_generator.py b/vulkan/scripts/driver_generator.py index 78b550c202..48c0ae9304 100644 --- a/vulkan/scripts/driver_generator.py +++ b/vulkan/scripts/driver_generator.py @@ -239,6 +239,8 @@ struct ProcHook { f.write(gencom.indent(2) + gencom.base_ext_name(ext) + ',\n') f.write('\n') + # EXTENSION_CORE_xxx API list must be the last set of enums after the extensions. + # This allows to easily identify "a" core function hook for version in gencom.version_code_list: f.write(gencom.indent(2) + 'EXTENSION_CORE_' + version + ',\n') |