diff options
107 files changed, 2543 insertions, 1266 deletions
diff --git a/cmds/atrace/atrace.cpp b/cmds/atrace/atrace.cpp index cedff0ba8a..79419d3ae6 100644 --- a/cmds/atrace/atrace.cpp +++ b/cmds/atrace/atrace.cpp @@ -323,9 +323,6 @@ static const char* k_funcgraphCpuPath = static const char* k_funcgraphProcPath = "options/funcgraph-proc"; -static const char* k_funcgraphFlatPath = - "options/funcgraph-flat"; - static const char* k_ftraceFilterPath = "set_ftrace_filter"; @@ -703,7 +700,6 @@ static bool setKernelTraceFuncs(const char* funcs) ok &= setKernelOptionEnable(k_funcgraphAbsTimePath, true); ok &= setKernelOptionEnable(k_funcgraphCpuPath, true); ok &= setKernelOptionEnable(k_funcgraphProcPath, true); - ok &= setKernelOptionEnable(k_funcgraphFlatPath, true); // Set the requested filter functions. ok &= truncateFile(k_ftraceFilterPath); diff --git a/cmds/dumpstate/Android.bp b/cmds/dumpstate/Android.bp index f48f1fb6f8..aff32c38c2 100644 --- a/cmds/dumpstate/Android.bp +++ b/cmds/dumpstate/Android.bp @@ -99,6 +99,7 @@ cc_defaults { "libhidlbase", "liblog", "libutils", + "libbinderdebug", ], srcs: [ "DumpstateService.cpp", diff --git a/cmds/dumpsys/Android.bp b/cmds/dumpsys/Android.bp index 91aa018451..6ab6b7f951 100644 --- a/cmds/dumpsys/Android.bp +++ b/cmds/dumpsys/Android.bp @@ -32,6 +32,7 @@ cc_defaults { "libutils", "liblog", "libbinder", + "libbinderdebug", ], static_libs: [ diff --git a/cmds/dumpsys/OWNERS b/cmds/dumpsys/OWNERS index 2a9b681318..4f6a89ebe5 100644 --- a/cmds/dumpsys/OWNERS +++ b/cmds/dumpsys/OWNERS @@ -2,3 +2,6 @@ set noparent nandana@google.com jsharkey@android.com + +# for ServiceManager mock +per-file dumpsys_test.cpp=smoreland@google.com diff --git a/cmds/dumpsys/dumpsys.cpp b/cmds/dumpsys/dumpsys.cpp index a017246184..ba1c449dbf 100644 --- a/cmds/dumpsys/dumpsys.cpp +++ b/cmds/dumpsys/dumpsys.cpp @@ -25,6 +25,7 @@ #include <binder/Parcel.h> #include <binder/ProcessState.h> #include <binder/TextOutput.h> +#include <binderdebug/BinderDebug.h> #include <serviceutils/PriorityDumper.h> #include <utils/Log.h> #include <utils/Vector.h> @@ -60,13 +61,15 @@ static void usage() { "usage: dumpsys\n" " To dump all services.\n" "or:\n" - " dumpsys [-t TIMEOUT] [--priority LEVEL] [--pid] [--help | -l | --skip SERVICES " + " dumpsys [-t TIMEOUT] [--priority LEVEL] [--pid] [--thread] [--help | -l | " + "--skip SERVICES " "| SERVICE [ARGS]]\n" " --help: shows this help\n" " -l: only list services, do not dump them\n" " -t TIMEOUT_SEC: TIMEOUT to use in seconds instead of default 10 seconds\n" " -T TIMEOUT_MS: TIMEOUT to use in milliseconds instead of default 10 seconds\n" " --pid: dump PID instead of usual dump\n" + " --thread: dump thread usage instead of usual dump\n" " --proto: filter services that support dumping data in proto format. Dumps\n" " will be in proto format.\n" " --priority LEVEL: filter services based on specified priority\n" @@ -125,7 +128,8 @@ int Dumpsys::main(int argc, char* const argv[]) { Type type = Type::DUMP; int timeoutArgMs = 10000; int priorityFlags = IServiceManager::DUMP_FLAG_PRIORITY_ALL; - static struct option longOptions[] = {{"pid", no_argument, 0, 0}, + static struct option longOptions[] = {{"thread", no_argument, 0, 0}, + {"pid", no_argument, 0, 0}, {"priority", required_argument, 0, 0}, {"proto", no_argument, 0, 0}, {"skip", no_argument, 0, 0}, @@ -163,6 +167,8 @@ int Dumpsys::main(int argc, char* const argv[]) { } } else if (!strcmp(longOptions[optionIndex].name, "pid")) { type = Type::PID; + } else if (!strcmp(longOptions[optionIndex].name, "thread")) { + type = Type::THREAD; } break; @@ -329,6 +335,23 @@ static status_t dumpPidToFd(const sp<IBinder>& service, const unique_fd& fd) { return OK; } +static status_t dumpThreadsToFd(const sp<IBinder>& service, const unique_fd& fd) { + pid_t pid; + status_t status = service->getDebugPid(&pid); + if (status != OK) { + return status; + } + BinderPidInfo pidInfo; + status = getBinderPidInfo(BinderDebugContext::BINDER, pid, &pidInfo); + if (status != OK) { + return status; + } + WriteStringToFd("Threads in use: " + std::to_string(pidInfo.threadUsage) + "/" + + std::to_string(pidInfo.threadCount) + "\n", + fd.get()); + return OK; +} + status_t Dumpsys::startDumpThread(Type type, const String16& serviceName, const Vector<String16>& args) { sp<IBinder> service = sm_->checkService(serviceName); @@ -359,6 +382,9 @@ status_t Dumpsys::startDumpThread(Type type, const String16& serviceName, case Type::PID: err = dumpPidToFd(service, remote_end); break; + case Type::THREAD: + err = dumpThreadsToFd(service, remote_end); + break; default: std::cerr << "Unknown dump type" << static_cast<int>(type) << std::endl; return; diff --git a/cmds/dumpsys/dumpsys.h b/cmds/dumpsys/dumpsys.h index 929c55c364..349947ce12 100644 --- a/cmds/dumpsys/dumpsys.h +++ b/cmds/dumpsys/dumpsys.h @@ -52,13 +52,14 @@ class Dumpsys { static void setServiceArgs(Vector<String16>& args, bool asProto, int priorityFlags); enum class Type { - DUMP, // dump using `dump` function - PID, // dump pid of server only + DUMP, // dump using `dump` function + PID, // dump pid of server only + THREAD, // dump thread usage of server only }; /** * Starts a thread to connect to a service and get its dump output. The thread redirects - * the output to a pipe. Thread must be stopped by a subsequent callto {@code + * the output to a pipe. Thread must be stopped by a subsequent call to {@code * stopDumpThread}. * @param serviceName * @param args list of arguments to pass to service dump method. diff --git a/cmds/dumpsys/tests/Android.bp b/cmds/dumpsys/tests/Android.bp index 6854c7550e..58fec30c9b 100644 --- a/cmds/dumpsys/tests/Android.bp +++ b/cmds/dumpsys/tests/Android.bp @@ -19,6 +19,7 @@ cc_test { "libbase", "libbinder", "libutils", + "libbinderdebug", ], static_libs: [ @@ -26,6 +27,4 @@ cc_test { "libgmock", "libserviceutils", ], - - clang: true, } diff --git a/cmds/dumpsys/tests/AndroidTest.xml b/cmds/dumpsys/tests/AndroidTest.xml index 1a8c67f7aa..c2351d9aff 100644 --- a/cmds/dumpsys/tests/AndroidTest.xml +++ b/cmds/dumpsys/tests/AndroidTest.xml @@ -23,4 +23,4 @@ <option name="native-test-device-path" value="/data/local/tmp" /> <option name="module-name" value="dumpsys_test" /> </test> -</configuration>
\ No newline at end of file +</configuration> diff --git a/cmds/dumpsys/tests/dumpsys_test.cpp b/cmds/dumpsys/tests/dumpsys_test.cpp index 67a77f6015..c9d2dbb883 100644 --- a/cmds/dumpsys/tests/dumpsys_test.cpp +++ b/cmds/dumpsys/tests/dumpsys_test.cpp @@ -16,12 +16,15 @@ #include "../dumpsys.h" +#include <regex> #include <vector> #include <gmock/gmock.h> #include <gtest/gtest.h> #include <android-base/file.h> +#include <binder/Binder.h> +#include <binder/ProcessState.h> #include <serviceutils/PriorityDumper.h> #include <utils/String16.h> #include <utils/String8.h> @@ -56,6 +59,7 @@ class ServiceManagerMock : public IServiceManager { MOCK_METHOD1(waitForService, sp<IBinder>(const String16&)); MOCK_METHOD1(isDeclared, bool(const String16&)); MOCK_METHOD1(getDeclaredInstances, Vector<String16>(const String16&)); + MOCK_METHOD1(updatableViaApex, std::optional<String16>(const String16&)); protected: MOCK_METHOD0(onAsBinder, IBinder*()); }; @@ -222,6 +226,10 @@ class DumpsysTest : public Test { EXPECT_THAT(stdout_, HasSubstr(expected)); } + void AssertOutputFormat(const std::string format) { + EXPECT_THAT(stdout_, testing::MatchesRegex(format)); + } + void AssertDumped(const std::string& service, const std::string& dump) { EXPECT_THAT(stdout_, HasSubstr("DUMP OF SERVICE " + service + ":\n" + dump)); EXPECT_THAT(stdout_, HasSubstr("was the duration of dumpsys " + service + ", ending at: ")); @@ -574,6 +582,30 @@ TEST_F(DumpsysTest, ListServiceWithPid) { AssertOutput(std::to_string(getpid()) + "\n"); } +// Tests 'dumpsys --thread' +TEST_F(DumpsysTest, ListAllServicesWithThread) { + ExpectListServices({"Locksmith", "Valet"}); + ExpectCheckService("Locksmith"); + ExpectCheckService("Valet"); + + CallMain({"--thread"}); + + AssertRunningServices({"Locksmith", "Valet"}); + + const std::string format("(.|\n)*((Threads in use: [0-9]+/[0-9]+)?\n-(.|\n)*){2}"); + AssertOutputFormat(format); +} + +// Tests 'dumpsys --thread service_name' +TEST_F(DumpsysTest, ListServiceWithThread) { + ExpectCheckService("Locksmith"); + + CallMain({"--thread", "Locksmith"}); + // returns an empty string without root enabled + const std::string format("(^$|Threads in use: [0-9]/[0-9]+\n)"); + AssertOutputFormat(format); +} + TEST_F(DumpsysTest, GetBytesWritten) { const char* serviceName = "service2"; const char* dumpContents = "dump1"; @@ -599,3 +631,13 @@ TEST_F(DumpsysTest, WriteDumpWithoutThreadStart) { /* as_proto = */ false, elapsedDuration, bytesWritten); EXPECT_THAT(status, Eq(INVALID_OPERATION)); } + +int main(int argc, char** argv) { + ::testing::InitGoogleTest(&argc, argv); + + // start a binder thread pool for testing --thread option + android::ProcessState::self()->setThreadPoolMaxThreadCount(8); + ProcessState::self()->startThreadPool(); + + return RUN_ALL_TESTS(); +} diff --git a/cmds/lshal/ListCommand.cpp b/cmds/lshal/ListCommand.cpp index d5110f6203..2722e214e8 100644 --- a/cmds/lshal/ListCommand.cpp +++ b/cmds/lshal/ListCommand.cpp @@ -417,7 +417,7 @@ void ListCommand::dumpVintf(const NullableOStream<std::ostream>& out) const { } } out << "-->" << std::endl; - out << vintf::gHalManifestConverter(manifest, vintf::SerializeFlags::HALS_ONLY); + out << vintf::toXml(manifest, vintf::SerializeFlags::HALS_ONLY); } std::string ListCommand::INIT_VINTF_NOTES{ diff --git a/cmds/lshal/test.cpp b/cmds/lshal/test.cpp index 7c1ca91528..6f08f74690 100644 --- a/cmds/lshal/test.cpp +++ b/cmds/lshal/test.cpp @@ -47,8 +47,6 @@ using ::android::hardware::hidl_vec; using ::android::hardware::Void; using android::vintf::Arch; using android::vintf::CompatibilityMatrix; -using android::vintf::gCompatibilityMatrixConverter; -using android::vintf::gHalManifestConverter; using android::vintf::HalManifest; using android::vintf::Transport; using android::vintf::VintfObject; @@ -510,7 +508,7 @@ TEST_F(ListTest, DumpVintf) { std::string error; vintf::HalManifest m; - EXPECT_EQ(true, vintf::gHalManifestConverter(&m, out.str(), &error)) + EXPECT_EQ(true, vintf::fromXml(&m, out.str(), &error)) << "--init-vintf does not emit valid HAL manifest: " << error; } @@ -775,10 +773,10 @@ TEST_F(ListTest, Vintf) { auto deviceMatrix = std::make_shared<CompatibilityMatrix>(); auto frameworkMatrix = std::make_shared<CompatibilityMatrix>(); - ASSERT_TRUE(gHalManifestConverter(deviceManifest.get(), deviceManifestXml)); - ASSERT_TRUE(gHalManifestConverter(frameworkManifest.get(), frameworkManifestXml)); - ASSERT_TRUE(gCompatibilityMatrixConverter(deviceMatrix.get(), deviceMatrixXml)); - ASSERT_TRUE(gCompatibilityMatrixConverter(frameworkMatrix.get(), frameworkMatrixXml)); + ASSERT_TRUE(fromXml(deviceManifest.get(), deviceManifestXml)); + ASSERT_TRUE(fromXml(frameworkManifest.get(), frameworkManifestXml)); + ASSERT_TRUE(fromXml(deviceMatrix.get(), deviceMatrixXml)); + ASSERT_TRUE(fromXml(frameworkMatrix.get(), frameworkMatrixXml)); ON_CALL(*mockList, getDeviceManifest()).WillByDefault(Return(deviceManifest)); ON_CALL(*mockList, getDeviceMatrix()).WillByDefault(Return(deviceMatrix)); @@ -964,7 +962,7 @@ public: " </hal>\n" "</manifest>"; auto manifest = std::make_shared<HalManifest>(); - EXPECT_TRUE(gHalManifestConverter(manifest.get(), mockManifestXml)); + EXPECT_TRUE(fromXml(manifest.get(), mockManifestXml)); EXPECT_CALL(*mockList, getDeviceManifest()) .Times(AnyNumber()) .WillRepeatedly(Return(manifest)); diff --git a/cmds/servicemanager/ServiceManager.cpp b/cmds/servicemanager/ServiceManager.cpp index 2f5524940e..b429fb3440 100644 --- a/cmds/servicemanager/ServiceManager.cpp +++ b/cmds/servicemanager/ServiceManager.cpp @@ -58,22 +58,34 @@ static bool forEachManifest(const std::function<bool(const ManifestWithDescripti return false; } -static bool isVintfDeclared(const std::string& name) { - size_t firstSlash = name.find('/'); - size_t lastDot = name.rfind('.', firstSlash); - if (firstSlash == std::string::npos || lastDot == std::string::npos) { - LOG(ERROR) << "VINTF HALs require names in the format type/instance (e.g. " - << "some.package.foo.IFoo/default) but got: " << name; - return false; +struct AidlName { + std::string package; + std::string iface; + std::string instance; + + static bool fill(const std::string& name, AidlName* aname) { + size_t firstSlash = name.find('/'); + size_t lastDot = name.rfind('.', firstSlash); + if (firstSlash == std::string::npos || lastDot == std::string::npos) { + LOG(ERROR) << "VINTF HALs require names in the format type/instance (e.g. " + << "some.package.foo.IFoo/default) but got: " << name; + return false; + } + aname->package = name.substr(0, lastDot); + aname->iface = name.substr(lastDot + 1, firstSlash - lastDot - 1); + aname->instance = name.substr(firstSlash + 1); + return true; } - const std::string package = name.substr(0, lastDot); - const std::string iface = name.substr(lastDot+1, firstSlash-lastDot-1); - const std::string instance = name.substr(firstSlash+1); +}; + +static bool isVintfDeclared(const std::string& name) { + AidlName aname; + if (!AidlName::fill(name, &aname)) return false; - bool found = forEachManifest([&] (const ManifestWithDescription& mwd) { - if (mwd.manifest->hasAidlInstance(package, iface, instance)) { + bool found = forEachManifest([&](const ManifestWithDescription& mwd) { + if (mwd.manifest->hasAidlInstance(aname.package, aname.iface, aname.instance)) { LOG(INFO) << "Found " << name << " in " << mwd.description << " VINTF manifest."; - return true; + return true; // break } return false; // continue }); @@ -81,13 +93,34 @@ static bool isVintfDeclared(const std::string& name) { if (!found) { // Although it is tested, explicitly rebuilding qualified name, in case it // becomes something unexpected. - LOG(ERROR) << "Could not find " << package << "." << iface << "/" << instance - << " in the VINTF manifest."; + LOG(ERROR) << "Could not find " << aname.package << "." << aname.iface << "/" + << aname.instance << " in the VINTF manifest."; } return found; } +static std::optional<std::string> getVintfUpdatableApex(const std::string& name) { + AidlName aname; + if (!AidlName::fill(name, &aname)) return std::nullopt; + + std::optional<std::string> updatableViaApex; + + forEachManifest([&](const ManifestWithDescription& mwd) { + mwd.manifest->forEachInstance([&](const auto& manifestInstance) { + if (manifestInstance.format() != vintf::HalFormat::AIDL) return true; + if (manifestInstance.package() != aname.package) return true; + if (manifestInstance.interface() != aname.iface) return true; + if (manifestInstance.instance() != aname.instance) return true; + updatableViaApex = manifestInstance.updatableViaApex(); + return false; // break (libvintf uses opposite convention) + }); + return false; // continue + }); + + return updatableViaApex; +} + static std::vector<std::string> getVintfInstances(const std::string& interface) { size_t lastDot = interface.rfind('.'); if (lastDot == std::string::npos) { @@ -388,6 +421,22 @@ binder::Status ServiceManager::getDeclaredInstances(const std::string& interface return Status::ok(); } +Status ServiceManager::updatableViaApex(const std::string& name, + std::optional<std::string>* outReturn) { + auto ctx = mAccess->getCallingContext(); + + if (!mAccess->canFind(ctx, name)) { + return Status::fromExceptionCode(Status::EX_SECURITY); + } + + *outReturn = std::nullopt; + +#ifndef VENDORSERVICEMANAGER + *outReturn = getVintfUpdatableApex(name); +#endif + return Status::ok(); +} + void ServiceManager::removeRegistrationCallback(const wp<IBinder>& who, ServiceCallbackMap::iterator* it, bool* found) { diff --git a/cmds/servicemanager/ServiceManager.h b/cmds/servicemanager/ServiceManager.h index c0891152e6..4f23c21078 100644 --- a/cmds/servicemanager/ServiceManager.h +++ b/cmds/servicemanager/ServiceManager.h @@ -46,6 +46,8 @@ public: binder::Status isDeclared(const std::string& name, bool* outReturn) override; binder::Status getDeclaredInstances(const std::string& interface, std::vector<std::string>* outReturn) override; + binder::Status updatableViaApex(const std::string& name, + std::optional<std::string>* outReturn) override; binder::Status registerClientCallback(const std::string& name, const sp<IBinder>& service, const sp<IClientCallback>& cb) override; binder::Status tryUnregisterService(const std::string& name, const sp<IBinder>& binder) override; diff --git a/include/android/surface_control.h b/include/android/surface_control.h index c5e3587517..b7eafcd6cd 100644 --- a/include/android/surface_control.h +++ b/include/android/surface_control.h @@ -147,6 +147,28 @@ typedef struct ASurfaceTransactionStats ASurfaceTransactionStats; typedef void (*ASurfaceTransaction_OnComplete)(void* context, ASurfaceTransactionStats* stats) __INTRODUCED_IN(29); + +/** + * The ASurfaceTransaction_OnCommit callback is invoked when transaction is applied and the updates + * are ready to be presented. This callback will be invoked before the + * ASurfaceTransaction_OnComplete callback. + * + * \param context Optional context provided by the client that is passed into the callback. + * + * \param stats Opaque handle that can be passed to ASurfaceTransactionStats functions to query + * information about the transaction. The handle is only valid during the callback. + * Present and release fences are not available for this callback. Querying them using + * ASurfaceTransactionStats_getPresentFenceFd and ASurfaceTransactionStats_getPreviousReleaseFenceFd + * will result in failure. + * + * THREADING + * The transaction committed callback can be invoked on any thread. + * + * Available since API level 31. + */ +typedef void (*ASurfaceTransaction_OnCommit)(void* context, ASurfaceTransactionStats* stats) + __INTRODUCED_IN(31); + /** * Returns the timestamp of when the frame was latched by the framework. Once a frame is * latched by the framework, it is presented at the following hardware vsync. @@ -161,6 +183,8 @@ int64_t ASurfaceTransactionStats_getLatchTime(ASurfaceTransactionStats* surface_ * The recipient of the callback takes ownership of the fence and is responsible for closing * it. If a device does not support present fences, a -1 will be returned. * + * This query is not valid for ASurfaceTransaction_OnCommit callback. + * * Available since API level 29. */ int ASurfaceTransactionStats_getPresentFenceFd(ASurfaceTransactionStats* surface_transaction_stats) @@ -218,6 +242,8 @@ int64_t ASurfaceTransactionStats_getAcquireTime(ASurfaceTransactionStats* surfac * The client must ensure that all pending refs on a buffer are released before attempting to reuse * this buffer, otherwise synchronization errors may occur. * + * This query is not valid for ASurfaceTransaction_OnCommit callback. + * * Available since API level 29. */ int ASurfaceTransactionStats_getPreviousReleaseFenceFd( @@ -236,6 +262,16 @@ void ASurfaceTransaction_setOnComplete(ASurfaceTransaction* transaction, void* c ASurfaceTransaction_OnComplete func) __INTRODUCED_IN(29); /** + * Sets the callback that will be invoked when the updates from this transaction are applied and are + * ready to be presented. This callback will be invoked before the ASurfaceTransaction_OnComplete + * callback. + * + * Available since API level 31. + */ +void ASurfaceTransaction_setOnCommit(ASurfaceTransaction* transaction, void* context, + ASurfaceTransaction_OnCommit func) __INTRODUCED_IN(31); + +/** * Reparents the \a surface_control from its old parent to the \a new_parent surface control. * Any children of the reparented \a surface_control will remain children of the \a surface_control. * @@ -329,39 +365,53 @@ void ASurfaceTransaction_setGeometry(ASurfaceTransaction* transaction, __INTRODUCED_IN(29); /** - * \param source The sub-rect within the buffer's content to be rendered inside the surface's area - * The surface's source rect is clipped by the bounds of its current buffer. The source rect's width - * and height must be > 0. + * Bounds the surface and its children to the bounds specified. The crop and buffer size will be + * used to determine the bounds of the surface. If no crop is specified and the surface has no + * buffer, the surface bounds is only constrained by the size of its parent bounds. + * + * \param crop The bounds of the crop to apply. * * Available since API level 31. */ -void ASurfaceTransaction_setSourceRect(ASurfaceTransaction* transaction, - ASurfaceControl* surface_control, const ARect& source) +void ASurfaceTransaction_setCrop(ASurfaceTransaction* transaction, + ASurfaceControl* surface_control, const ARect& crop) __INTRODUCED_IN(31); /** - * \param destination Specifies the rect in the parent's space where this surface will be drawn. The - * post source rect bounds are scaled to fit the destination rect. The surface's destination rect is - * clipped by the bounds of its parent. The destination rect's width and height must be > 0. + * Specifies the position in the parent's space where the surface will be drawn. + * + * \param x The x position to render the surface. + * \param y The y position to render the surface. * * Available since API level 31. */ void ASurfaceTransaction_setPosition(ASurfaceTransaction* transaction, - ASurfaceControl* surface_control, const ARect& destination) + ASurfaceControl* surface_control, int32_t x, int32_t y) __INTRODUCED_IN(31); /** * \param transform The transform applied after the source rect is applied to the buffer. This - * parameter should be set to 0 for no transform. To specify a transfrom use the + * parameter should be set to 0 for no transform. To specify a transform use the * NATIVE_WINDOW_TRANSFORM_* enum. * * Available since API level 31. */ -void ASurfaceTransaction_setTransform(ASurfaceTransaction* transaction, +void ASurfaceTransaction_setBufferTransform(ASurfaceTransaction* transaction, ASurfaceControl* surface_control, int32_t transform) __INTRODUCED_IN(31); /** + * Sets an x and y scale of a surface with (0, 0) as the centerpoint of the scale. + * + * \param xScale The scale in the x direction. Must be greater than 0. + * \param yScale The scale in the y direction. Must be greater than 0. + * + * Available since API level 31. + */ +void ASurfaceTransaction_setScale(ASurfaceTransaction* transaction, + ASurfaceControl* surface_control, float xScale, float yScale) + __INTRODUCED_IN(31); +/** * Parameter for ASurfaceTransaction_setBufferTransparency(). */ enum { diff --git a/libs/binder/IPCThreadState.cpp b/libs/binder/IPCThreadState.cpp index 406bd54e6f..6fb1227f63 100644 --- a/libs/binder/IPCThreadState.cpp +++ b/libs/binder/IPCThreadState.cpp @@ -90,6 +90,8 @@ static const char *kReturnStrings[] = { "BR_DEAD_BINDER", "BR_CLEAR_DEATH_NOTIFICATION_DONE", "BR_FAILED_REPLY", + "BR_FROZEN_REPLY", + "BR_ONEWAY_SPAM_SUSPECT", "BR_TRANSACTION_SEC_CTX", }; @@ -894,6 +896,11 @@ status_t IPCThreadState::waitForResponse(Parcel *reply, status_t *acquireResult) } switch (cmd) { + case BR_ONEWAY_SPAM_SUSPECT: + ALOGE("Process seems to be sending too many oneway calls."); + CallStack::logStack("oneway spamming", CallStack::getCurrent().get(), + ANDROID_LOG_ERROR); + [[fallthrough]]; case BR_TRANSACTION_COMPLETE: if (!reply && !acquireResult) goto finish; break; diff --git a/libs/binder/IServiceManager.cpp b/libs/binder/IServiceManager.cpp index 61f4581df3..f684cf672f 100644 --- a/libs/binder/IServiceManager.cpp +++ b/libs/binder/IServiceManager.cpp @@ -75,6 +75,7 @@ public: sp<IBinder> waitForService(const String16& name16) override; bool isDeclared(const String16& name) override; Vector<String16> getDeclaredInstances(const String16& interface) override; + std::optional<String16> updatableViaApex(const String16& name) override; // for legacy ABI const String16& getInterfaceDescriptor() const override { @@ -388,4 +389,12 @@ Vector<String16> ServiceManagerShim::getDeclaredInstances(const String16& interf return res; } +std::optional<String16> ServiceManagerShim::updatableViaApex(const String16& name) { + std::optional<std::string> declared; + if (!mTheRealServiceManager->updatableViaApex(String8(name).c_str(), &declared).isOk()) { + return std::nullopt; + } + return declared ? std::optional<String16>(String16(declared.value().c_str())) : std::nullopt; +} + } // namespace android diff --git a/libs/binder/ProcessState.cpp b/libs/binder/ProcessState.cpp index 7647a8c0df..1d3beb4736 100644 --- a/libs/binder/ProcessState.cpp +++ b/libs/binder/ProcessState.cpp @@ -43,6 +43,7 @@ #define BINDER_VM_SIZE ((1 * 1024 * 1024) - sysconf(_SC_PAGE_SIZE) * 2) #define DEFAULT_MAX_BINDER_THREADS 15 +#define DEFAULT_ENABLE_ONEWAY_SPAM_DETECTION 1 #ifdef __ANDROID_VNDK__ const char* kDefaultDriver = "/dev/vndbinder"; @@ -358,6 +359,15 @@ status_t ProcessState::setThreadPoolMaxThreadCount(size_t maxThreads) { return result; } +status_t ProcessState::enableOnewaySpamDetection(bool enable) { + uint32_t enableDetection = enable ? 1 : 0; + if (ioctl(mDriverFD, BINDER_ENABLE_ONEWAY_SPAM_DETECTION, &enableDetection) == -1) { + ALOGI("Binder ioctl to enable oneway spam detection failed: %s", strerror(errno)); + return -errno; + } + return NO_ERROR; +} + void ProcessState::giveThreadPoolName() { androidSetThreadName( makeBinderThreadName().string() ); } @@ -388,6 +398,11 @@ static int open_driver(const char *driver) if (result == -1) { ALOGE("Binder ioctl to set max threads failed: %s", strerror(errno)); } + uint32_t enable = DEFAULT_ENABLE_ONEWAY_SPAM_DETECTION; + result = ioctl(fd, BINDER_ENABLE_ONEWAY_SPAM_DETECTION, &enable); + if (result == -1) { + ALOGI("Binder ioctl to enable oneway spam detection failed: %s", strerror(errno)); + } } else { ALOGW("Opening '%s' failed: %s\n", driver, strerror(errno)); } diff --git a/libs/binder/RpcConnection.cpp b/libs/binder/RpcConnection.cpp index 4aff92b58a..1388a801e4 100644 --- a/libs/binder/RpcConnection.cpp +++ b/libs/binder/RpcConnection.cpp @@ -18,6 +18,16 @@ #include <binder/RpcConnection.h> +#include <arpa/inet.h> +#include <netdb.h> +#include <netinet/in.h> +#include <sys/socket.h> +#include <sys/types.h> +#include <sys/un.h> +#include <unistd.h> + +#include <string_view> + #include <binder/Parcel.h> #include <binder/Stability.h> #include <utils/String8.h> @@ -25,11 +35,6 @@ #include "RpcState.h" #include "RpcWireFormat.h" -#include <sys/socket.h> -#include <sys/types.h> -#include <sys/un.h> -#include <unistd.h> - #ifdef __GLIBC__ extern "C" pid_t gettid(); #endif @@ -41,6 +46,7 @@ extern "C" pid_t gettid(); namespace android { using base::unique_fd; +using AddrInfo = std::unique_ptr<addrinfo, decltype(&freeaddrinfo)>; RpcConnection::SocketAddress::~SocketAddress() {} @@ -51,6 +57,10 @@ RpcConnection::RpcConnection() { } RpcConnection::~RpcConnection() { LOG_RPC_DETAIL("RpcConnection destroyed %p", this); + + std::lock_guard<std::mutex> _l(mSocketMutex); + LOG_ALWAYS_FATAL_IF(mServers.size() != 0, + "Should not be able to destroy a connection with servers in use."); } sp<RpcConnection> RpcConnection::make() { @@ -120,6 +130,70 @@ bool RpcConnection::addVsockClient(unsigned int cid, unsigned int port) { #endif // __BIONIC__ +class SocketAddressImpl : public RpcConnection::SocketAddress { +public: + SocketAddressImpl(const sockaddr* addr, size_t size, const String8& desc) + : mAddr(addr), mSize(size), mDesc(desc) {} + [[nodiscard]] std::string toString() const override { + return std::string(mDesc.c_str(), mDesc.size()); + } + [[nodiscard]] const sockaddr* addr() const override { return mAddr; } + [[nodiscard]] size_t addrSize() const override { return mSize; } + void set(const sockaddr* addr, size_t size) { + mAddr = addr; + mSize = size; + } + +private: + const sockaddr* mAddr = nullptr; + size_t mSize = 0; + String8 mDesc; +}; + +AddrInfo GetAddrInfo(const char* addr, unsigned int port) { + addrinfo hint{ + .ai_flags = 0, + .ai_family = AF_UNSPEC, + .ai_socktype = SOCK_STREAM, + .ai_protocol = 0, + }; + addrinfo* aiStart = nullptr; + if (int rc = getaddrinfo(addr, std::to_string(port).data(), &hint, &aiStart); 0 != rc) { + ALOGE("Unable to resolve %s:%u: %s", addr, port, gai_strerror(rc)); + return AddrInfo(nullptr, nullptr); + } + if (aiStart == nullptr) { + ALOGE("Unable to resolve %s:%u: getaddrinfo returns null", addr, port); + return AddrInfo(nullptr, nullptr); + } + return AddrInfo(aiStart, &freeaddrinfo); +} + +bool RpcConnection::setupInetServer(unsigned int port) { + auto aiStart = GetAddrInfo("127.0.0.1", port); + if (aiStart == nullptr) return false; + SocketAddressImpl socketAddress(nullptr, 0, String8::format("127.0.0.1:%u", port)); + for (auto ai = aiStart.get(); ai != nullptr; ai = ai->ai_next) { + socketAddress.set(ai->ai_addr, ai->ai_addrlen); + if (setupSocketServer(socketAddress)) return true; + } + ALOGE("None of the socket address resolved for 127.0.0.1:%u can be set up as inet server.", + port); + return false; +} + +bool RpcConnection::addInetClient(const char* addr, unsigned int port) { + auto aiStart = GetAddrInfo(addr, port); + if (aiStart == nullptr) return false; + SocketAddressImpl socketAddress(nullptr, 0, String8::format("%s:%u", addr, port)); + for (auto ai = aiStart.get(); ai != nullptr; ai = ai->ai_next) { + socketAddress.set(ai->ai_addr, ai->ai_addrlen); + if (addSocketClient(socketAddress)) return true; + } + ALOGE("None of the socket address resolved for %s:%u can be added as inet client.", addr, port); + return false; +} + bool RpcConnection::addNullDebuggingClient() { unique_fd serverFd(TEMP_FAILURE_RETRY(open("/dev/null", O_WRONLY | O_CLOEXEC))); @@ -152,36 +226,35 @@ status_t RpcConnection::sendDecStrong(const RpcAddress& address) { } void RpcConnection::join() { - // establish a connection - { - unique_fd clientFd( - TEMP_FAILURE_RETRY(accept4(mServer.get(), nullptr, 0 /*length*/, SOCK_CLOEXEC))); - if (clientFd < 0) { - // If this log becomes confusing, should save more state from setupUnixDomainServer - // in order to output here. - ALOGE("Could not accept4 socket: %s", strerror(errno)); - return; - } - - LOG_RPC_DETAIL("accept4 on fd %d yields fd %d", mServer.get(), clientFd.get()); - - assignServerToThisThread(std::move(clientFd)); + // TODO(b/185167543): do this dynamically, instead of from a static number + // of threads + unique_fd clientFd( + TEMP_FAILURE_RETRY(accept4(mServer.get(), nullptr, 0 /*length*/, SOCK_CLOEXEC))); + if (clientFd < 0) { + // If this log becomes confusing, should save more state from setupUnixDomainServer + // in order to output here. + ALOGE("Could not accept4 socket: %s", strerror(errno)); + return; } - // We may not use the connection we just established (two threads might - // establish connections for each other), but for now, just use one - // server/socket connection. - ExclusiveSocket socket(sp<RpcConnection>::fromExisting(this), SocketUse::SERVER); + LOG_RPC_DETAIL("accept4 on fd %d yields fd %d", mServer.get(), clientFd.get()); + + // must be registered to allow arbitrary client code executing commands to + // be able to do nested calls (we can't only read from it) + sp<ConnectionSocket> socket = assignServerToThisThread(std::move(clientFd)); while (true) { status_t error = - state()->getAndExecuteCommand(socket.fd(), sp<RpcConnection>::fromExisting(this)); + state()->getAndExecuteCommand(socket->fd, sp<RpcConnection>::fromExisting(this)); if (error != OK) { ALOGI("Binder socket thread closing w/ status %s", statusToString(error).c_str()); - return; + break; } } + + LOG_ALWAYS_FATAL_IF(!removeServerSocket(socket), + "bad state: socket object guaranteed to be in list"); } void RpcConnection::setForServer(const wp<RpcServer>& server) { @@ -246,11 +319,23 @@ void RpcConnection::addClient(unique_fd&& fd) { mClients.push_back(connection); } -void RpcConnection::assignServerToThisThread(unique_fd&& fd) { +sp<RpcConnection::ConnectionSocket> RpcConnection::assignServerToThisThread(unique_fd&& fd) { std::lock_guard<std::mutex> _l(mSocketMutex); sp<ConnectionSocket> connection = sp<ConnectionSocket>::make(); connection->fd = std::move(fd); + connection->exclusiveTid = gettid(); mServers.push_back(connection); + + return connection; +} + +bool RpcConnection::removeServerSocket(const sp<ConnectionSocket>& socket) { + std::lock_guard<std::mutex> _l(mSocketMutex); + if (auto it = std::find(mServers.begin(), mServers.end(), socket); it != mServers.end()) { + mServers.erase(it); + return true; + } + return false; } RpcConnection::ExclusiveSocket::ExclusiveSocket(const sp<RpcConnection>& connection, SocketUse use) @@ -265,37 +350,31 @@ RpcConnection::ExclusiveSocket::ExclusiveSocket(const sp<RpcConnection>& connect // CHECK FOR DEDICATED CLIENT SOCKET // - // A server/looper should always use a dedicated connection. - if (use != SocketUse::SERVER) { - findSocket(tid, &exclusive, &available, mConnection->mClients, - mConnection->mClientsOffset); - - // WARNING: this assumes a server cannot request its client to send - // a transaction, as mServers is excluded below. - // - // Imagine we have more than one thread in play, and a single thread - // sends a synchronous, then an asynchronous command. Imagine the - // asynchronous command is sent on the first client socket. Then, if - // we naively send a synchronous command to that same socket, the - // thread on the far side might be busy processing the asynchronous - // command. So, we move to considering the second available thread - // for subsequent calls. - if (use == SocketUse::CLIENT_ASYNC && (exclusive != nullptr || available != nullptr)) { - mConnection->mClientsOffset = - (mConnection->mClientsOffset + 1) % mConnection->mClients.size(); - } + // A server/looper should always use a dedicated connection if available + findSocket(tid, &exclusive, &available, mConnection->mClients, mConnection->mClientsOffset); + + // WARNING: this assumes a server cannot request its client to send + // a transaction, as mServers is excluded below. + // + // Imagine we have more than one thread in play, and a single thread + // sends a synchronous, then an asynchronous command. Imagine the + // asynchronous command is sent on the first client socket. Then, if + // we naively send a synchronous command to that same socket, the + // thread on the far side might be busy processing the asynchronous + // command. So, we move to considering the second available thread + // for subsequent calls. + if (use == SocketUse::CLIENT_ASYNC && (exclusive != nullptr || available != nullptr)) { + mConnection->mClientsOffset = + (mConnection->mClientsOffset + 1) % mConnection->mClients.size(); } - // USE SERVING SOCKET (to start serving or for nested transaction) + // USE SERVING SOCKET (for nested transaction) // // asynchronous calls cannot be nested if (use != SocketUse::CLIENT_ASYNC) { - // servers should start serving on an available thread only - // otherwise, this should only be a nested call - bool useAvailable = use == SocketUse::SERVER; - - findSocket(tid, &exclusive, (useAvailable ? &available : nullptr), - mConnection->mServers, 0 /* index hint */); + // server sockets are always assigned to a thread + findSocket(tid, &exclusive, nullptr /*available*/, mConnection->mServers, + 0 /* index hint */); } // if our thread is already using a connection, prioritize using that @@ -309,8 +388,6 @@ RpcConnection::ExclusiveSocket::ExclusiveSocket(const sp<RpcConnection>& connect break; } - LOG_ALWAYS_FATAL_IF(use == SocketUse::SERVER, "Must create connection to join one."); - // in regular binder, this would usually be a deadlock :) LOG_ALWAYS_FATAL_IF(mConnection->mClients.size() == 0, "Not a client of any connection. You must create a connection to an " diff --git a/libs/binder/aidl/android/os/IServiceManager.aidl b/libs/binder/aidl/android/os/IServiceManager.aidl index 2fabf947cd..75c4092559 100644 --- a/libs/binder/aidl/android/os/IServiceManager.aidl +++ b/libs/binder/aidl/android/os/IServiceManager.aidl @@ -108,6 +108,11 @@ interface IServiceManager { @utf8InCpp String[] getDeclaredInstances(@utf8InCpp String iface); /** + * If updatable-via-apex, returns the APEX via which this is updated. + */ + @nullable @utf8InCpp String updatableViaApex(@utf8InCpp String name); + + /** * Request a callback when the number of clients of the service changes. * Used by LazyServiceRegistrar to dynamically stop services that have no clients. */ diff --git a/libs/binder/include/binder/IServiceManager.h b/libs/binder/include/binder/IServiceManager.h index 5f0d056c5d..3dbe2c49f4 100644 --- a/libs/binder/include/binder/IServiceManager.h +++ b/libs/binder/include/binder/IServiceManager.h @@ -20,6 +20,8 @@ #include <utils/Vector.h> #include <utils/String16.h> +#include <optional> + namespace android { // ---------------------------------------------------------------------- @@ -99,6 +101,12 @@ public: * Get all instances of a service as declared in the VINTF manifest */ virtual Vector<String16> getDeclaredInstances(const String16& interface) = 0; + + /** + * If this instance is updatable via an APEX, returns the APEX with which + * this can be updated. + */ + virtual std::optional<String16> updatableViaApex(const String16& name) = 0; }; sp<IServiceManager> defaultServiceManager(); diff --git a/libs/binder/include/binder/ProcessState.h b/libs/binder/include/binder/ProcessState.h index 0919648541..b9db5d726c 100644 --- a/libs/binder/include/binder/ProcessState.h +++ b/libs/binder/include/binder/ProcessState.h @@ -58,6 +58,7 @@ public: void spawnPooledThread(bool isMain); status_t setThreadPoolMaxThreadCount(size_t maxThreads); + status_t enableOnewaySpamDetection(bool enable); void giveThreadPoolName(); String8 getDriverName(); diff --git a/libs/binder/include/binder/RpcConnection.h b/libs/binder/include/binder/RpcConnection.h index dba47b4d15..2395e78de2 100644 --- a/libs/binder/include/binder/RpcConnection.h +++ b/libs/binder/include/binder/RpcConnection.h @@ -74,6 +74,16 @@ public: #endif // __BIONIC__ /** + * Creates an RPC server at the current port. + */ + [[nodiscard]] bool setupInetServer(unsigned int port); + + /** + * Connects to an RPC server at the given address and port. + */ + [[nodiscard]] bool addInetClient(const char* addr, unsigned int port); + + /** * For debugging! * * Sets up an empty socket. All queries to this socket which require a @@ -118,11 +128,6 @@ private: friend sp<RpcConnection>; RpcConnection(); - bool setupSocketServer(const SocketAddress& address); - bool addSocketClient(const SocketAddress& address); - void addClient(base::unique_fd&& fd); - void assignServerToThisThread(base::unique_fd&& fd); - struct ConnectionSocket : public RefBase { base::unique_fd fd; @@ -131,11 +136,16 @@ private: std::optional<pid_t> exclusiveTid; }; + bool setupSocketServer(const SocketAddress& address); + bool addSocketClient(const SocketAddress& address); + void addClient(base::unique_fd&& fd); + sp<ConnectionSocket> assignServerToThisThread(base::unique_fd&& fd); + bool removeServerSocket(const sp<ConnectionSocket>& socket); + enum class SocketUse { CLIENT, CLIENT_ASYNC, CLIENT_REFCOUNT, - SERVER, }; // RAII object for connection socket diff --git a/libs/binder/include/private/binder/binder_module.h b/libs/binder/include/private/binder/binder_module.h index 157919903f..151235c16b 100644 --- a/libs/binder/include/private/binder/binder_module.h +++ b/libs/binder/include/private/binder/binder_module.h @@ -32,10 +32,6 @@ #include <sys/ioctl.h> #include <linux/android/binder.h> -#ifdef __cplusplus -namespace android { -#endif - #ifndef BR_FROZEN_REPLY // Temporary definition of BR_FROZEN_REPLY. For production // this will come from UAPI binder.h @@ -88,8 +84,18 @@ struct binder_frozen_status_info { }; #endif //BINDER_GET_FROZEN_INFO -#ifdef __cplusplus -} // namespace android -#endif +#ifndef BR_ONEWAY_SPAM_SUSPECT +// Temporary definition of BR_ONEWAY_SPAM_SUSPECT. For production +// this will come from UAPI binder.h +#define BR_ONEWAY_SPAM_SUSPECT _IO('r', 19) +#endif //BR_ONEWAY_SPAM_SUSPECT + +#ifndef BINDER_ENABLE_ONEWAY_SPAM_DETECTION +/* + * Temporary definitions for oneway spam detection support. For the final version + * these will be defined in the UAPI binder.h file from upstream kernel. + */ +#define BINDER_ENABLE_ONEWAY_SPAM_DETECTION _IOW('b', 16, __u32) +#endif //BINDER_ENABLE_ONEWAY_SPAM_DETECTION #endif // _BINDER_MODULE_H_ diff --git a/libs/binder/ndk/include_platform/android/binder_manager.h b/libs/binder/ndk/include_platform/android/binder_manager.h index 55169140df..a90b4aabca 100644 --- a/libs/binder/ndk/include_platform/android/binder_manager.h +++ b/libs/binder/ndk/include_platform/android/binder_manager.h @@ -124,6 +124,15 @@ void AServiceManager_forEachDeclaredInstance(const char* interface, void* contex __INTRODUCED_IN(31); /** + * Check if a service is updatable via an APEX module. + * + * \param instance identifier of the service + * + * \return whether the interface is updatable via APEX + */ +bool AServiceManager_isUpdatableViaApex(const char* instance) __INTRODUCED_IN(31); + +/** * Prevent lazy services without client from shutting down their process * * \param persist 'true' if the process should not exit. diff --git a/libs/binder/ndk/libbinder_ndk.map.txt b/libs/binder/ndk/libbinder_ndk.map.txt index 67c85b66d4..7d4b82e4b6 100644 --- a/libs/binder/ndk/libbinder_ndk.map.txt +++ b/libs/binder/ndk/libbinder_ndk.map.txt @@ -118,14 +118,15 @@ LIBBINDER_NDK31 { # introduced=31 AIBinder_getCallingSid; # apex AIBinder_setRequestingSid; # apex AParcel_markSensitive; # llndk - AServiceManager_isDeclared; # apex llndk AServiceManager_forEachDeclaredInstance; # apex llndk - AServiceManager_registerLazyService; # llndk - AServiceManager_waitForService; # apex llndk AServiceManager_forceLazyServicesPersist; # llndk + AServiceManager_isDeclared; # apex llndk + AServiceManager_isUpdatableViaApex; # apex + AServiceManager_reRegister; # llndk + AServiceManager_registerLazyService; # llndk AServiceManager_setActiveServicesCallback; # llndk AServiceManager_tryUnregister; # llndk - AServiceManager_reRegister; # llndk + AServiceManager_waitForService; # apex llndk AIBinder_forceDowngradeToSystemStability; # apex AIBinder_forceDowngradeToVendorStability; # llndk diff --git a/libs/binder/ndk/service_manager.cpp b/libs/binder/ndk/service_manager.cpp index 1ccd0d2a2b..7649a26ff6 100644 --- a/libs/binder/ndk/service_manager.cpp +++ b/libs/binder/ndk/service_manager.cpp @@ -105,6 +105,14 @@ void AServiceManager_forEachDeclaredInstance(const char* interface, void* contex callback(String8(instance).c_str(), context); } } +bool AServiceManager_isUpdatableViaApex(const char* instance) { + if (instance == nullptr) { + return false; + } + + sp<IServiceManager> sm = defaultServiceManager(); + return sm->updatableViaApex(String16(instance)) != std::nullopt; +} void AServiceManager_forceLazyServicesPersist(bool persist) { auto serviceRegistrar = android::binder::LazyServiceRegistrar::getInstance(); serviceRegistrar.forcePersist(persist); diff --git a/libs/binder/ndk/tests/libbinder_ndk_unit_test.cpp b/libs/binder/ndk/tests/libbinder_ndk_unit_test.cpp index 6a88401962..62db3cfbd2 100644 --- a/libs/binder/ndk/tests/libbinder_ndk_unit_test.cpp +++ b/libs/binder/ndk/tests/libbinder_ndk_unit_test.cpp @@ -300,6 +300,11 @@ TEST(NdkBinder, GetLazyService) { EXPECT_EQ(STATUS_OK, AIBinder_ping(binder.get())); } +TEST(NdkBinder, IsUpdatable) { + bool isUpdatable = AServiceManager_isUpdatableViaApex("android.hardware.light.ILights/default"); + EXPECT_EQ(isUpdatable, false); +} + // This is too slow TEST(NdkBinder, CheckLazyServiceShutDown) { ndk::SpAIBinder binder(AServiceManager_waitForService(kLazyBinderNdkUnitTestService)); diff --git a/libs/binder/rust/src/binder.rs b/libs/binder/rust/src/binder.rs index 321b422185..695a83e414 100644 --- a/libs/binder/rust/src/binder.rs +++ b/libs/binder/rust/src/binder.rs @@ -548,6 +548,28 @@ unsafe impl<T, V: AsNative<T>> AsNative<T> for Option<V> { } } +/// The features to enable when creating a native Binder. +/// +/// This should always be initialised with a default value, e.g.: +/// ``` +/// # use binder::BinderFeatures; +/// BinderFeatures { +/// set_requesting_sid: true, +/// ..BinderFeatures::default(), +/// } +/// ``` +#[derive(Clone, Debug, Default, Eq, PartialEq)] +pub struct BinderFeatures { + /// Indicates that the service intends to receive caller security contexts. This must be true + /// for `ThreadState::with_calling_sid` to work. + pub set_requesting_sid: bool, + // Ensure that clients include a ..BinderFeatures::default() to preserve backwards compatibility + // when new fields are added. #[non_exhaustive] doesn't work because it prevents struct + // expressions entirely. + #[doc(hidden)] + pub _non_exhaustive: (), +} + /// Declare typed interfaces for a binder object. /// /// Given an interface trait and descriptor string, create a native and remote @@ -730,8 +752,9 @@ macro_rules! declare_binder_interface { impl $native { /// Create a new binder service. - pub fn new_binder<T: $interface + Sync + Send + 'static>(inner: T) -> $crate::Strong<dyn $interface> { - let binder = $crate::Binder::new_with_stability($native(Box::new(inner)), $stability); + pub fn new_binder<T: $interface + Sync + Send + 'static>(inner: T, features: $crate::BinderFeatures) -> $crate::Strong<dyn $interface> { + let mut binder = $crate::Binder::new_with_stability($native(Box::new(inner)), $stability); + $crate::IBinderInternal::set_requesting_sid(&mut binder, features.set_requesting_sid); $crate::Strong::new(Box::new(binder)) } } diff --git a/libs/binder/rust/src/lib.rs b/libs/binder/rust/src/lib.rs index 30928a5cff..2694cba870 100644 --- a/libs/binder/rust/src/lib.rs +++ b/libs/binder/rust/src/lib.rs @@ -107,10 +107,9 @@ use binder_ndk_sys as sys; pub mod parcel; pub use crate::binder::{ - FromIBinder, IBinder, IBinderInternal, Interface, InterfaceClass, Remotable, - Stability, Strong, TransactionCode, TransactionFlags, Weak, - FIRST_CALL_TRANSACTION, FLAG_CLEAR_BUF, FLAG_ONEWAY, FLAG_PRIVATE_LOCAL, - LAST_CALL_TRANSACTION, + BinderFeatures, FromIBinder, IBinder, IBinderInternal, Interface, InterfaceClass, Remotable, + Stability, Strong, TransactionCode, TransactionFlags, Weak, FIRST_CALL_TRANSACTION, + FLAG_CLEAR_BUF, FLAG_ONEWAY, FLAG_PRIVATE_LOCAL, LAST_CALL_TRANSACTION, }; pub use error::{status_t, ExceptionCode, Result, Status, StatusCode}; pub use native::add_service; @@ -125,8 +124,8 @@ pub mod public_api { pub use super::parcel::ParcelFileDescriptor; pub use super::{add_service, get_interface}; pub use super::{ - DeathRecipient, ExceptionCode, IBinder, Interface, ProcessState, SpIBinder, Status, - StatusCode, Strong, ThreadState, Weak, WpIBinder, + BinderFeatures, DeathRecipient, ExceptionCode, IBinder, Interface, ProcessState, SpIBinder, + Status, StatusCode, Strong, ThreadState, Weak, WpIBinder, }; /// Binder result containing a [`Status`] on error. diff --git a/libs/binder/rust/tests/integration.rs b/libs/binder/rust/tests/integration.rs index 60e3502759..03320076cb 100644 --- a/libs/binder/rust/tests/integration.rs +++ b/libs/binder/rust/tests/integration.rs @@ -19,7 +19,7 @@ use binder::declare_binder_interface; use binder::parcel::Parcel; use binder::{ - Binder, IBinderInternal, Interface, StatusCode, ThreadState, TransactionCode, + Binder, BinderFeatures, IBinderInternal, Interface, StatusCode, ThreadState, TransactionCode, FIRST_CALL_TRANSACTION, }; use std::convert::{TryFrom, TryInto}; @@ -55,7 +55,8 @@ fn main() -> Result<(), &'static str> { }))); service.set_requesting_sid(true); if let Some(extension_name) = extension_name { - let extension = BnTest::new_binder(TestService { s: extension_name }); + let extension = + BnTest::new_binder(TestService { s: extension_name }, BinderFeatures::default()); service .set_extension(&mut extension.as_binder()) .expect("Could not add extension"); @@ -212,8 +213,8 @@ mod tests { use std::time::Duration; use binder::{ - Binder, DeathRecipient, FromIBinder, IBinder, IBinderInternal, Interface, SpIBinder, - StatusCode, Strong, + Binder, BinderFeatures, DeathRecipient, FromIBinder, IBinder, IBinderInternal, Interface, + SpIBinder, StatusCode, Strong, }; use super::{BnTest, ITest, ITestSameDescriptor, TestService, RUST_SERVICE_BINARY}; @@ -495,9 +496,12 @@ mod tests { #[test] fn reassociate_rust_binder() { let service_name = "testing_service"; - let service_ibinder = BnTest::new_binder(TestService { - s: service_name.to_string(), - }) + let service_ibinder = BnTest::new_binder( + TestService { + s: service_name.to_string(), + }, + BinderFeatures::default(), + ) .as_binder(); let service: Strong<dyn ITest> = service_ibinder @@ -510,9 +514,12 @@ mod tests { #[test] fn weak_binder_upgrade() { let service_name = "testing_service"; - let service = BnTest::new_binder(TestService { - s: service_name.to_string(), - }); + let service = BnTest::new_binder( + TestService { + s: service_name.to_string(), + }, + BinderFeatures::default(), + ); let weak = Strong::downgrade(&service); @@ -525,9 +532,12 @@ mod tests { fn weak_binder_upgrade_dead() { let service_name = "testing_service"; let weak = { - let service = BnTest::new_binder(TestService { - s: service_name.to_string(), - }); + let service = BnTest::new_binder( + TestService { + s: service_name.to_string(), + }, + BinderFeatures::default(), + ); Strong::downgrade(&service) }; @@ -538,9 +548,12 @@ mod tests { #[test] fn weak_binder_clone() { let service_name = "testing_service"; - let service = BnTest::new_binder(TestService { - s: service_name.to_string(), - }); + let service = BnTest::new_binder( + TestService { + s: service_name.to_string(), + }, + BinderFeatures::default(), + ); let weak = Strong::downgrade(&service); let cloned = weak.clone(); @@ -556,12 +569,18 @@ mod tests { #[test] #[allow(clippy::eq_op)] fn binder_ord() { - let service1 = BnTest::new_binder(TestService { - s: "testing_service1".to_string(), - }); - let service2 = BnTest::new_binder(TestService { - s: "testing_service2".to_string(), - }); + let service1 = BnTest::new_binder( + TestService { + s: "testing_service1".to_string(), + }, + BinderFeatures::default(), + ); + let service2 = BnTest::new_binder( + TestService { + s: "testing_service2".to_string(), + }, + BinderFeatures::default(), + ); assert!(!(service1 < service1)); assert!(!(service1 > service1)); diff --git a/libs/binder/rust/tests/ndk_rust_interop.rs b/libs/binder/rust/tests/ndk_rust_interop.rs index ce75ab7125..4702e45f1f 100644 --- a/libs/binder/rust/tests/ndk_rust_interop.rs +++ b/libs/binder/rust/tests/ndk_rust_interop.rs @@ -16,15 +16,13 @@ //! Rust Binder NDK interop tests -use std::ffi::CStr; -use std::os::raw::{c_char, c_int}; -use ::IBinderRustNdkInteropTest::binder::{self, Interface, StatusCode}; use ::IBinderRustNdkInteropTest::aidl::IBinderRustNdkInteropTest::{ BnBinderRustNdkInteropTest, IBinderRustNdkInteropTest, }; -use ::IBinderRustNdkInteropTest::aidl::IBinderRustNdkInteropTestOther::{ - IBinderRustNdkInteropTestOther, -}; +use ::IBinderRustNdkInteropTest::aidl::IBinderRustNdkInteropTestOther::IBinderRustNdkInteropTestOther; +use ::IBinderRustNdkInteropTest::binder::{self, BinderFeatures, Interface, StatusCode}; +use std::ffi::CStr; +use std::os::raw::{c_char, c_int}; /// Look up the provided AIDL service and call its echo method. /// @@ -37,18 +35,21 @@ pub unsafe extern "C" fn rust_call_ndk(service_name: *const c_char) -> c_int { // The Rust class descriptor pointer will not match the NDK one, but the // descriptor strings match so this needs to still associate. - let service: binder::Strong<dyn IBinderRustNdkInteropTest> = match binder::get_interface(service_name) { - Err(e) => { - eprintln!("Could not find Ndk service {}: {:?}", service_name, e); - return StatusCode::NAME_NOT_FOUND as c_int; - } - Ok(service) => service, - }; + let service: binder::Strong<dyn IBinderRustNdkInteropTest> = + match binder::get_interface(service_name) { + Err(e) => { + eprintln!("Could not find Ndk service {}: {:?}", service_name, e); + return StatusCode::NAME_NOT_FOUND as c_int; + } + Ok(service) => service, + }; match service.echo("testing") { - Ok(s) => if s != "testing" { - return StatusCode::BAD_VALUE as c_int; - }, + Ok(s) => { + if s != "testing" { + return StatusCode::BAD_VALUE as c_int; + } + } Err(e) => return e.into(), } @@ -88,7 +89,7 @@ impl IBinderRustNdkInteropTest for Service { #[no_mangle] pub unsafe extern "C" fn rust_start_service(service_name: *const c_char) -> c_int { let service_name = CStr::from_ptr(service_name).to_str().unwrap(); - let service = BnBinderRustNdkInteropTest::new_binder(Service); + let service = BnBinderRustNdkInteropTest::new_binder(Service, BinderFeatures::default()); match binder::add_service(&service_name, service.as_binder()) { Ok(_) => StatusCode::OK as c_int, Err(e) => e as c_int, diff --git a/libs/binder/rust/tests/serialization.rs b/libs/binder/rust/tests/serialization.rs index f1b068ee43..66ba846c2d 100644 --- a/libs/binder/rust/tests/serialization.rs +++ b/libs/binder/rust/tests/serialization.rs @@ -18,11 +18,11 @@ //! access. use binder::declare_binder_interface; +use binder::parcel::ParcelFileDescriptor; use binder::{ - Binder, ExceptionCode, Interface, Parcel, Result, SpIBinder, Status, + Binder, BinderFeatures, ExceptionCode, Interface, Parcel, Result, SpIBinder, Status, StatusCode, TransactionCode, }; -use binder::parcel::ParcelFileDescriptor; use std::ffi::{c_void, CStr, CString}; use std::sync::Once; @@ -85,7 +85,7 @@ static mut SERVICE: Option<SpIBinder> = None; pub extern "C" fn rust_service() -> *mut c_void { unsafe { SERVICE_ONCE.call_once(|| { - SERVICE = Some(BnReadParcelTest::new_binder(()).as_binder()); + SERVICE = Some(BnReadParcelTest::new_binder((), BinderFeatures::default()).as_binder()); }); SERVICE.as_ref().unwrap().as_raw().cast() } @@ -108,8 +108,12 @@ impl ReadParcelTest for BpReadParcelTest {} impl ReadParcelTest for () {} #[allow(clippy::float_cmp)] -fn on_transact(_service: &dyn ReadParcelTest, code: TransactionCode, - parcel: &Parcel, reply: &mut Parcel) -> Result<()> { +fn on_transact( + _service: &dyn ReadParcelTest, + code: TransactionCode, + parcel: &Parcel, + reply: &mut Parcel, +) -> Result<()> { match code { bindings::Transaction_TEST_BOOL => { assert_eq!(parcel.read::<bool>()?, true); diff --git a/libs/binder/tests/binderLibTest.cpp b/libs/binder/tests/binderLibTest.cpp index e2193fadab..dc8c0f157e 100644 --- a/libs/binder/tests/binderLibTest.cpp +++ b/libs/binder/tests/binderLibTest.cpp @@ -88,6 +88,7 @@ enum BinderLibTestTranscationCode { BINDER_LIB_TEST_GETPID, BINDER_LIB_TEST_ECHO_VECTOR, BINDER_LIB_TEST_REJECT_BUF, + BINDER_LIB_TEST_CAN_GET_SID, }; pid_t start_server_process(int arg2, bool usePoll = false) @@ -1192,6 +1193,14 @@ TEST_F(BinderLibTest, BufRejected) { EXPECT_NE(NO_ERROR, ret); } +TEST_F(BinderLibTest, GotSid) { + sp<IBinder> server = addServer(); + + Parcel data; + status_t ret = server->transact(BINDER_LIB_TEST_CAN_GET_SID, data, nullptr); + EXPECT_EQ(OK, ret); +} + class BinderLibTestService : public BBinder { public: @@ -1494,6 +1503,9 @@ class BinderLibTestService : public BBinder case BINDER_LIB_TEST_REJECT_BUF: { return data.objectsCount() == 0 ? BAD_VALUE : NO_ERROR; } + case BINDER_LIB_TEST_CAN_GET_SID: { + return IPCThreadState::self()->getCallingSid() == nullptr ? BAD_VALUE : NO_ERROR; + } default: return UNKNOWN_TRANSACTION; }; diff --git a/libs/binder/tests/binderRpcTest.cpp b/libs/binder/tests/binderRpcTest.cpp index 33402937ab..dd68fdbc6d 100644 --- a/libs/binder/tests/binderRpcTest.cpp +++ b/libs/binder/tests/binderRpcTest.cpp @@ -80,11 +80,10 @@ public: sp<RpcConnection> connection; Status sendString(const std::string& str) override { - std::cout << "Child received string: " << str << std::endl; + (void)str; return Status::ok(); } Status doubleString(const std::string& str, std::string* strstr) override { - std::cout << "Child received string to double: " << str << std::endl; *strstr = str + str; return Status::ok(); } @@ -257,6 +256,7 @@ enum class SocketType { #ifdef __BIONIC__ VSOCK, #endif // __BIONIC__ + INET, }; static inline std::string PrintSocketType(const testing::TestParamInfo<SocketType>& info) { switch (info.param) { @@ -266,6 +266,8 @@ static inline std::string PrintSocketType(const testing::TestParamInfo<SocketTyp case SocketType::VSOCK: return "vm_socket"; #endif // __BIONIC__ + case SocketType::INET: + return "inet_socket"; default: LOG_ALWAYS_FATAL("Unknown socket type"); return ""; @@ -305,6 +307,9 @@ public: CHECK(connection->setupVsockServer(port)); break; #endif // __BIONIC__ + case SocketType::INET: + CHECK(connection->setupInetServer(port)); + break; default: LOG_ALWAYS_FATAL("Unknown socket type"); } @@ -335,6 +340,9 @@ public: if (ret.connection->addVsockClient(VMADDR_CID_LOCAL, port)) goto success; break; #endif // __BIONIC__ + case SocketType::INET: + if (ret.connection->addInetClient("127.0.0.1", port)) goto success; + break; default: LOG_ALWAYS_FATAL("Unknown socket type"); } @@ -740,7 +748,7 @@ TEST_P(BinderRpc, ThreadingStressTest) { threads.push_back(std::thread([&] { for (size_t j = 0; j < kNumCalls; j++) { sp<IBinder> out; - proc.rootIface->repeatBinder(proc.rootBinder, &out); + EXPECT_OK(proc.rootIface->repeatBinder(proc.rootBinder, &out)); EXPECT_EQ(proc.rootBinder, out); } })); @@ -749,6 +757,28 @@ TEST_P(BinderRpc, ThreadingStressTest) { for (auto& t : threads) t.join(); } +TEST_P(BinderRpc, OnewayStressTest) { + constexpr size_t kNumClientThreads = 10; + constexpr size_t kNumServerThreads = 10; + constexpr size_t kNumCalls = 100; + + auto proc = createRpcTestSocketServerProcess(kNumServerThreads); + + std::vector<std::thread> threads; + for (size_t i = 0; i < kNumClientThreads; i++) { + threads.push_back(std::thread([&] { + for (size_t j = 0; j < kNumCalls; j++) { + EXPECT_OK(proc.rootIface->sendString("a")); + } + + // check threads are not stuck + EXPECT_OK(proc.rootIface->sleepMs(250)); + })); + } + + for (auto& t : threads) t.join(); +} + TEST_P(BinderRpc, OnewayCallDoesNotWait) { constexpr size_t kReallyLongTimeMs = 100; constexpr size_t kSleepMs = kReallyLongTimeMs * 5; @@ -852,12 +882,13 @@ TEST_P(BinderRpc, Fds) { } INSTANTIATE_TEST_CASE_P(PerSocket, BinderRpc, - ::testing::Values(SocketType::UNIX + ::testing::ValuesIn({ + SocketType::UNIX, #ifdef __BIONIC__ - , - SocketType::VSOCK + SocketType::VSOCK, #endif // __BIONIC__ - ), + SocketType::INET, + }), PrintSocketType); } // namespace android diff --git a/libs/binder/tests/binderStabilityTest.cpp b/libs/binder/tests/binderStabilityTest.cpp index cb309bdd81..2ce13df4b3 100644 --- a/libs/binder/tests/binderStabilityTest.cpp +++ b/libs/binder/tests/binderStabilityTest.cpp @@ -192,6 +192,8 @@ TEST(BinderStability, VintfStabilityServerMustBeDeclaredInManifest) { EXPECT_EQ(Status::EX_ILLEGAL_ARGUMENT, android::defaultServiceManager()->addService(String16("."), vintfServer)) << instance8; EXPECT_FALSE(android::defaultServiceManager()->isDeclared(instance)) << instance8; + EXPECT_EQ(std::nullopt, android::defaultServiceManager()->updatableViaApex(instance)) + << instance8; } } diff --git a/libs/fakeservicemanager/ServiceManager.cpp b/libs/fakeservicemanager/ServiceManager.cpp index 4ecbe531c2..761e45c967 100644 --- a/libs/fakeservicemanager/ServiceManager.cpp +++ b/libs/fakeservicemanager/ServiceManager.cpp @@ -73,4 +73,9 @@ Vector<String16> ServiceManager::getDeclaredInstances(const String16& name) { return out; } +std::optional<String16> ServiceManager::updatableViaApex(const String16& name) { + (void)name; + return std::nullopt; +} + } // namespace android diff --git a/libs/fakeservicemanager/ServiceManager.h b/libs/fakeservicemanager/ServiceManager.h index 4ef47fb043..e26c21b9e9 100644 --- a/libs/fakeservicemanager/ServiceManager.h +++ b/libs/fakeservicemanager/ServiceManager.h @@ -19,6 +19,7 @@ #include <binder/IServiceManager.h> #include <map> +#include <optional> namespace android { @@ -48,6 +49,8 @@ public: Vector<String16> getDeclaredInstances(const String16& iface) override; + std::optional<String16> updatableViaApex(const String16& name) override; + private: std::map<String16, sp<IBinder>> mNameToService; }; diff --git a/libs/gui/BLASTBufferQueue.cpp b/libs/gui/BLASTBufferQueue.cpp index dbc1a7ed22..3d854c2b92 100644 --- a/libs/gui/BLASTBufferQueue.cpp +++ b/libs/gui/BLASTBufferQueue.cpp @@ -186,6 +186,7 @@ BLASTBufferQueue::~BLASTBufferQueue() { void BLASTBufferQueue::update(const sp<SurfaceControl>& surface, uint32_t width, uint32_t height, int32_t format) { std::unique_lock _lock{mMutex}; + BQA_LOGV("update width=%d height=%d format=%d", width, height, format); if (mFormat != format) { mFormat = format; mBufferItemConsumer->setDefaultBufferFormat(convertBufferFormat(format)); @@ -397,10 +398,11 @@ void BLASTBufferQueue::processNextBufferLocked(bool useNextTransaction) { // Ensure BLASTBufferQueue stays alive until we receive the transaction complete callback. incStrong((void*)transactionCallbackThunk); + Rect crop = computeCrop(bufferItem); mLastAcquiredFrameNumber = bufferItem.mFrameNumber; mLastBufferInfo.update(true /* hasBuffer */, bufferItem.mGraphicBuffer->getWidth(), bufferItem.mGraphicBuffer->getHeight(), bufferItem.mTransform, - bufferItem.mScalingMode); + bufferItem.mScalingMode, crop); auto releaseBufferCallback = std::bind(releaseBufferCallbackThunk, wp<BLASTBufferQueue>(this) /* callbackContext */, @@ -415,7 +417,7 @@ void BLASTBufferQueue::processNextBufferLocked(bool useNextTransaction) { mSurfaceControlsWithPendingCallback.push(mSurfaceControl); setMatrix(t, mLastBufferInfo); - t->setCrop(mSurfaceControl, computeCrop(bufferItem)); + t->setCrop(mSurfaceControl, crop); t->setTransform(mSurfaceControl, bufferItem.mTransform); t->setTransformToDisplayInverse(mSurfaceControl, bufferItem.mTransformToDisplayInverse); if (!bufferItem.mIsAutoTimestamp) { @@ -543,13 +545,15 @@ bool BLASTBufferQueue::rejectBuffer(const BufferItem& item) { void BLASTBufferQueue::setMatrix(SurfaceComposerClient::Transaction* t, const BufferInfo& bufferInfo) { - uint32_t bufWidth = bufferInfo.width; - uint32_t bufHeight = bufferInfo.height; + uint32_t bufWidth = bufferInfo.crop.getWidth(); + uint32_t bufHeight = bufferInfo.crop.getHeight(); - float dsdx = mSize.width / static_cast<float>(bufWidth); - float dsdy = mSize.height / static_cast<float>(bufHeight); + float sx = mSize.width / static_cast<float>(bufWidth); + float sy = mSize.height / static_cast<float>(bufHeight); - t->setMatrix(mSurfaceControl, dsdx, 0, 0, dsdy); + t->setMatrix(mSurfaceControl, sx, 0, 0, sy); + // Update position based on crop. + t->setPosition(mSurfaceControl, bufferInfo.crop.left * sx * -1, bufferInfo.crop.top * sy * -1); } void BLASTBufferQueue::setTransactionCompleteCallback( diff --git a/libs/gui/ISurfaceComposer.cpp b/libs/gui/ISurfaceComposer.cpp index 454aa9eb7b..53721cf6f7 100644 --- a/libs/gui/ISurfaceComposer.cpp +++ b/libs/gui/ISurfaceComposer.cpp @@ -99,7 +99,7 @@ public: SAFE_PARCEL(data.writeVectorSize, listenerCallbacks); for (const auto& [listener, callbackIds] : listenerCallbacks) { SAFE_PARCEL(data.writeStrongBinder, listener); - SAFE_PARCEL(data.writeInt64Vector, callbackIds); + SAFE_PARCEL(data.writeParcelableVector, callbackIds); } SAFE_PARCEL(data.writeUint64, transactionId); @@ -497,6 +497,28 @@ public: return result; } + status_t onPullAtom(const int32_t atomId, std::string* pulledData, bool* success) { + Parcel data, reply; + SAFE_PARCEL(data.writeInterfaceToken, ISurfaceComposer::getInterfaceDescriptor()); + SAFE_PARCEL(data.writeInt32, atomId); + + status_t err = remote()->transact(BnSurfaceComposer::ON_PULL_ATOM, data, &reply); + if (err != NO_ERROR) { + ALOGE("onPullAtom failed to transact: %d", err); + return err; + } + + int32_t size = 0; + SAFE_PARCEL(reply.readInt32, &size); + const void* dataPtr = reply.readInplace(size); + if (dataPtr == nullptr) { + return UNEXPECTED_NULL; + } + pulledData->assign((const char*)dataPtr, size); + SAFE_PARCEL(reply.readBool, success); + return NO_ERROR; + } + status_t enableVSyncInjections(bool enable) override { Parcel data, reply; status_t result = data.writeInterfaceToken(ISurfaceComposer::getInterfaceDescriptor()); @@ -1246,7 +1268,7 @@ status_t BnSurfaceComposer::onTransact( for (int32_t i = 0; i < listenersSize; i++) { SAFE_PARCEL(data.readStrongBinder, &tmpBinder); std::vector<CallbackId> callbackIds; - SAFE_PARCEL(data.readInt64Vector, &callbackIds); + SAFE_PARCEL(data.readParcelableVector, &callbackIds); listenerCallbacks.emplace_back(tmpBinder, callbackIds); } @@ -2021,6 +2043,19 @@ status_t BnSurfaceComposer::onTransact( } return overrideHdrTypes(display, hdrTypesVector); } + case ON_PULL_ATOM: { + CHECK_INTERFACE(ISurfaceComposer, data, reply); + int32_t atomId = 0; + SAFE_PARCEL(data.readInt32, &atomId); + + std::string pulledData; + bool success; + status_t err = onPullAtom(atomId, &pulledData, &success); + SAFE_PARCEL(reply->writeByteArray, pulledData.size(), + reinterpret_cast<const uint8_t*>(pulledData.data())); + SAFE_PARCEL(reply->writeBool, success); + return err; + } default: { return BBinder::onTransact(code, data, reply, flags); } diff --git a/libs/gui/ITransactionCompletedListener.cpp b/libs/gui/ITransactionCompletedListener.cpp index b42793b8ac..f74f91e034 100644 --- a/libs/gui/ITransactionCompletedListener.cpp +++ b/libs/gui/ITransactionCompletedListener.cpp @@ -152,7 +152,7 @@ status_t SurfaceStats::readFromParcel(const Parcel* input) { } status_t TransactionStats::writeToParcel(Parcel* output) const { - status_t err = output->writeInt64Vector(callbackIds); + status_t err = output->writeParcelableVector(callbackIds); if (err != NO_ERROR) { return err; } @@ -176,7 +176,7 @@ status_t TransactionStats::writeToParcel(Parcel* output) const { } status_t TransactionStats::readFromParcel(const Parcel* input) { - status_t err = input->readInt64Vector(&callbackIds); + status_t err = input->readParcelableVector(&callbackIds); if (err != NO_ERROR) { return err; } @@ -227,8 +227,9 @@ status_t ListenerStats::readFromParcel(const Parcel* input) { return NO_ERROR; } -ListenerStats ListenerStats::createEmpty(const sp<IBinder>& listener, - const std::unordered_set<CallbackId>& callbackIds) { +ListenerStats ListenerStats::createEmpty( + const sp<IBinder>& listener, + const std::unordered_set<CallbackId, CallbackIdHash>& callbackIds) { ListenerStats listenerStats; listenerStats.listener = listener; listenerStats.transactionStats.emplace_back(callbackIds); @@ -278,4 +279,28 @@ status_t BnTransactionCompletedListener::onTransact(uint32_t code, const Parcel& } } +ListenerCallbacks ListenerCallbacks::filter(CallbackId::Type type) const { + std::vector<CallbackId> filteredCallbackIds; + for (const auto& callbackId : callbackIds) { + if (callbackId.type == type) { + filteredCallbackIds.push_back(callbackId); + } + } + return ListenerCallbacks(transactionCompletedListener, filteredCallbackIds); +} + +status_t CallbackId::writeToParcel(Parcel* output) const { + SAFE_PARCEL(output->writeInt64, id); + SAFE_PARCEL(output->writeInt32, static_cast<int32_t>(type)); + return NO_ERROR; +} + +status_t CallbackId::readFromParcel(const Parcel* input) { + SAFE_PARCEL(input->readInt64, &id); + int32_t typeAsInt; + SAFE_PARCEL(input->readInt32, &typeAsInt); + type = static_cast<CallbackId::Type>(typeAsInt); + return NO_ERROR; +} + }; // namespace android diff --git a/libs/gui/LayerState.cpp b/libs/gui/LayerState.cpp index 55ed7fe298..517b49e6b5 100644 --- a/libs/gui/LayerState.cpp +++ b/libs/gui/LayerState.cpp @@ -140,7 +140,7 @@ status_t layer_state_t::write(Parcel& output) const for (auto listener : listeners) { SAFE_PARCEL(output.writeStrongBinder, listener.transactionCompletedListener); - SAFE_PARCEL(output.writeInt64Vector, listener.callbackIds); + SAFE_PARCEL(output.writeParcelableVector, listener.callbackIds); } SAFE_PARCEL(output.writeFloat, shadowRadius); SAFE_PARCEL(output.writeInt32, frameRateSelectionPriority); @@ -258,7 +258,7 @@ status_t layer_state_t::read(const Parcel& input) sp<IBinder> listener; std::vector<CallbackId> callbackIds; SAFE_PARCEL(input.readNullableStrongBinder, &listener); - SAFE_PARCEL(input.readInt64Vector, &callbackIds); + SAFE_PARCEL(input.readParcelableVector, &callbackIds); listeners.emplace_back(listener, callbackIds); } SAFE_PARCEL(input.readFloat, &shadowRadius); diff --git a/libs/gui/SurfaceComposerClient.cpp b/libs/gui/SurfaceComposerClient.cpp index 6d198a105f..5db0eae416 100644 --- a/libs/gui/SurfaceComposerClient.cpp +++ b/libs/gui/SurfaceComposerClient.cpp @@ -139,7 +139,7 @@ JankDataListener::~JankDataListener() { // 0 is an invalid callback id TransactionCompletedListener::TransactionCompletedListener() : mCallbackIdCounter(1) {} -CallbackId TransactionCompletedListener::getNextIdLocked() { +int64_t TransactionCompletedListener::getNextIdLocked() { return mCallbackIdCounter++; } @@ -163,13 +163,13 @@ void TransactionCompletedListener::startListeningLocked() { CallbackId TransactionCompletedListener::addCallbackFunction( const TransactionCompletedCallback& callbackFunction, const std::unordered_set<sp<SurfaceControl>, SurfaceComposerClient::SCHash>& - surfaceControls) { + surfaceControls, + CallbackId::Type callbackType) { std::lock_guard<std::mutex> lock(mMutex); startListeningLocked(); - CallbackId callbackId = getNextIdLocked(); + CallbackId callbackId(getNextIdLocked(), callbackType); mCallbacks[callbackId].callbackFunction = callbackFunction; - auto& callbackSurfaceControls = mCallbacks[callbackId].surfaceControls; for (const auto& surfaceControl : surfaceControls) { @@ -228,7 +228,7 @@ void TransactionCompletedListener::removeSurfaceStatsListener(void* context, voi void TransactionCompletedListener::addSurfaceControlToCallbacks( const sp<SurfaceControl>& surfaceControl, - const std::unordered_set<CallbackId>& callbackIds) { + const std::unordered_set<CallbackId, CallbackIdHash>& callbackIds) { std::lock_guard<std::mutex> lock(mMutex); for (auto callbackId : callbackIds) { @@ -240,7 +240,7 @@ void TransactionCompletedListener::addSurfaceControlToCallbacks( } void TransactionCompletedListener::onTransactionCompleted(ListenerStats listenerStats) { - std::unordered_map<CallbackId, CallbackTranslation> callbacksMap; + std::unordered_map<CallbackId, CallbackTranslation, CallbackIdHash> callbacksMap; std::multimap<sp<IBinder>, sp<JankDataListener>> jankListenersMap; std::multimap<sp<IBinder>, SurfaceStatsCallbackEntry> surfaceListeners; { @@ -267,7 +267,36 @@ void TransactionCompletedListener::onTransactionCompleted(ListenerStats listener } } for (const auto& transactionStats : listenerStats.transactionStats) { + // handle on commit callbacks + for (auto callbackId : transactionStats.callbackIds) { + if (callbackId.type != CallbackId::Type::ON_COMMIT) { + continue; + } + auto& [callbackFunction, callbackSurfaceControls] = callbacksMap[callbackId]; + if (!callbackFunction) { + ALOGE("cannot call null callback function, skipping"); + continue; + } + std::vector<SurfaceControlStats> surfaceControlStats; + for (const auto& surfaceStats : transactionStats.surfaceStats) { + surfaceControlStats + .emplace_back(callbacksMap[callbackId] + .surfaceControls[surfaceStats.surfaceControl], + transactionStats.latchTime, surfaceStats.acquireTime, + transactionStats.presentFence, + surfaceStats.previousReleaseFence, surfaceStats.transformHint, + surfaceStats.eventStats); + } + + callbackFunction(transactionStats.latchTime, transactionStats.presentFence, + surfaceControlStats); + } + + // handle on complete callbacks for (auto callbackId : transactionStats.callbackIds) { + if (callbackId.type != CallbackId::Type::ON_COMPLETE) { + continue; + } auto& [callbackFunction, callbackSurfaceControls] = callbacksMap[callbackId]; if (!callbackFunction) { ALOGE("cannot call null callback function, skipping"); @@ -542,7 +571,9 @@ status_t SurfaceComposerClient::Transaction::readFromParcel(const Parcel* parcel return BAD_VALUE; } for (size_t j = 0; j < numCallbackIds; j++) { - listenerCallbacks[listener].callbackIds.insert(parcel->readInt64()); + CallbackId id; + parcel->readParcelable(&id); + listenerCallbacks[listener].callbackIds.insert(id); } size_t numSurfaces = parcel->readUint32(); if (numSurfaces > parcel->dataSize()) { @@ -628,7 +659,7 @@ status_t SurfaceComposerClient::Transaction::writeToParcel(Parcel* parcel) const parcel->writeStrongBinder(ITransactionCompletedListener::asBinder(listener)); parcel->writeUint32(static_cast<uint32_t>(callbackInfo.callbackIds.size())); for (auto callbackId : callbackInfo.callbackIds) { - parcel->writeInt64(callbackId); + parcel->writeParcelable(callbackId); } parcel->writeUint32(static_cast<uint32_t>(callbackInfo.surfaceControls.size())); for (auto surfaceControl : callbackInfo.surfaceControls) { @@ -1389,9 +1420,9 @@ SurfaceComposerClient::Transaction::setFrameRateSelectionPriority(const sp<Surfa return *this; } -SurfaceComposerClient::Transaction& -SurfaceComposerClient::Transaction::addTransactionCompletedCallback( - TransactionCompletedCallbackTakesContext callback, void* callbackContext) { +SurfaceComposerClient::Transaction& SurfaceComposerClient::Transaction::addTransactionCallback( + TransactionCompletedCallbackTakesContext callback, void* callbackContext, + CallbackId::Type callbackType) { auto listener = TransactionCompletedListener::getInstance(); auto callbackWithContext = std::bind(callback, callbackContext, std::placeholders::_1, @@ -1399,13 +1430,26 @@ SurfaceComposerClient::Transaction::addTransactionCompletedCallback( const auto& surfaceControls = mListenerCallbacks[TransactionCompletedListener::getIInstance()].surfaceControls; - CallbackId callbackId = listener->addCallbackFunction(callbackWithContext, surfaceControls); + CallbackId callbackId = + listener->addCallbackFunction(callbackWithContext, surfaceControls, callbackType); mListenerCallbacks[TransactionCompletedListener::getIInstance()].callbackIds.emplace( callbackId); return *this; } +SurfaceComposerClient::Transaction& +SurfaceComposerClient::Transaction::addTransactionCompletedCallback( + TransactionCompletedCallbackTakesContext callback, void* callbackContext) { + return addTransactionCallback(callback, callbackContext, CallbackId::Type::ON_COMPLETE); +} + +SurfaceComposerClient::Transaction& +SurfaceComposerClient::Transaction::addTransactionCommittedCallback( + TransactionCompletedCallbackTakesContext callback, void* callbackContext) { + return addTransactionCallback(callback, callbackContext, CallbackId::Type::ON_COMMIT); +} + SurfaceComposerClient::Transaction& SurfaceComposerClient::Transaction::notifyProducerDisconnect( const sp<SurfaceControl>& sc) { layer_state_t* s = getLayerState(sc); @@ -1796,7 +1840,8 @@ status_t SurfaceComposerClient::createSurfaceChecked(const String8& name, uint32 } ALOGE_IF(err, "SurfaceComposerClient::createSurface error %s", strerror(-err)); if (err == NO_ERROR) { - *outSurface = new SurfaceControl(this, handle, gbp, id, w, h, format, transformHint); + *outSurface = + new SurfaceControl(this, handle, gbp, id, w, h, format, transformHint, flags); } } return err; @@ -1949,6 +1994,11 @@ status_t SurfaceComposerClient::overrideHdrTypes(const sp<IBinder>& display, return ComposerService::getComposerService()->overrideHdrTypes(display, hdrTypes); } +status_t SurfaceComposerClient::onPullAtom(const int32_t atomId, std::string* outData, + bool* success) { + return ComposerService::getComposerService()->onPullAtom(atomId, outData, success); +} + status_t SurfaceComposerClient::getDisplayedContentSamplingAttributes(const sp<IBinder>& display, ui::PixelFormat* outFormat, ui::Dataspace* outDataspace, diff --git a/libs/gui/include/gui/BLASTBufferQueue.h b/libs/gui/include/gui/BLASTBufferQueue.h index 8a23223660..139dbb7f6b 100644 --- a/libs/gui/include/gui/BLASTBufferQueue.h +++ b/libs/gui/include/gui/BLASTBufferQueue.h @@ -151,14 +151,20 @@ private: // we get the next buffer. This will support scenarios where the layer can change sizes // and the buffer will scale to fit the new size. uint32_t scalingMode = NATIVE_WINDOW_SCALING_MODE_SCALE_TO_WINDOW; + Rect crop; void update(bool hasBuffer, uint32_t width, uint32_t height, uint32_t transform, - uint32_t scalingMode) { + uint32_t scalingMode, const Rect& crop) { this->hasBuffer = hasBuffer; this->width = width; this->height = height; this->transform = transform; this->scalingMode = scalingMode; + if (!crop.isEmpty()) { + this->crop = crop; + } else { + this->crop = Rect(width, height); + } } }; diff --git a/libs/gui/include/gui/ISurfaceComposer.h b/libs/gui/include/gui/ISurfaceComposer.h index ccd6d4e754..cb0468901a 100644 --- a/libs/gui/include/gui/ISurfaceComposer.h +++ b/libs/gui/include/gui/ISurfaceComposer.h @@ -275,6 +275,12 @@ public: virtual status_t overrideHdrTypes(const sp<IBinder>& display, const std::vector<ui::Hdr>& hdrTypes) = 0; + /* Pulls surfaceflinger atoms global stats and layer stats to pipe to statsd. + * + * Requires the calling uid be from system server. + */ + virtual status_t onPullAtom(const int32_t atomId, std::string* outData, bool* success) = 0; + virtual status_t enableVSyncInjections(bool enable) = 0; virtual status_t injectVSync(nsecs_t when) = 0; @@ -600,6 +606,7 @@ public: OVERRIDE_HDR_TYPES, ADD_HDR_LAYER_INFO_LISTENER, REMOVE_HDR_LAYER_INFO_LISTENER, + ON_PULL_ATOM, // Always append new enum to the end. }; diff --git a/libs/gui/include/gui/ITransactionCompletedListener.h b/libs/gui/include/gui/ITransactionCompletedListener.h index 098760e89d..2d71194f70 100644 --- a/libs/gui/include/gui/ITransactionCompletedListener.h +++ b/libs/gui/include/gui/ITransactionCompletedListener.h @@ -36,7 +36,22 @@ namespace android { class ITransactionCompletedListener; class ListenerCallbacks; -using CallbackId = int64_t; +class CallbackId : public Parcelable { +public: + int64_t id; + enum class Type : int32_t { ON_COMPLETE, ON_COMMIT } type; + + CallbackId() {} + CallbackId(int64_t id, Type type) : id(id), type(type) {} + status_t writeToParcel(Parcel* output) const override; + status_t readFromParcel(const Parcel* input) override; + + bool operator==(const CallbackId& rhs) const { return id == rhs.id && type == rhs.type; } +}; + +struct CallbackIdHash { + std::size_t operator()(const CallbackId& key) const { return std::hash<int64_t>()(key.id); } +}; class FrameEventHistoryStats : public Parcelable { public: @@ -112,7 +127,7 @@ public: TransactionStats() = default; TransactionStats(const std::vector<CallbackId>& ids) : callbackIds(ids) {} - TransactionStats(const std::unordered_set<CallbackId>& ids) + TransactionStats(const std::unordered_set<CallbackId, CallbackIdHash>& ids) : callbackIds(ids.begin(), ids.end()) {} TransactionStats(const std::vector<CallbackId>& ids, nsecs_t latch, const sp<Fence>& present, const std::vector<SurfaceStats>& surfaces) @@ -129,8 +144,9 @@ public: status_t writeToParcel(Parcel* output) const override; status_t readFromParcel(const Parcel* input) override; - static ListenerStats createEmpty(const sp<IBinder>& listener, - const std::unordered_set<CallbackId>& callbackIds); + static ListenerStats createEmpty( + const sp<IBinder>& listener, + const std::unordered_set<CallbackId, CallbackIdHash>& callbackIds); sp<IBinder> listener; std::vector<TransactionStats> transactionStats; @@ -156,7 +172,8 @@ public: class ListenerCallbacks { public: - ListenerCallbacks(const sp<IBinder>& listener, const std::unordered_set<CallbackId>& callbacks) + ListenerCallbacks(const sp<IBinder>& listener, + const std::unordered_set<CallbackId, CallbackIdHash>& callbacks) : transactionCompletedListener(listener), callbackIds(callbacks.begin(), callbacks.end()) {} @@ -170,9 +187,12 @@ public: if (callbackIds.empty()) { return rhs.callbackIds.empty(); } - return callbackIds.front() == rhs.callbackIds.front(); + return callbackIds.front().id == rhs.callbackIds.front().id; } + // Returns a new ListenerCallbacks filtered by type + ListenerCallbacks filter(CallbackId::Type type) const; + sp<IBinder> transactionCompletedListener; std::vector<CallbackId> callbackIds; }; @@ -191,7 +211,7 @@ struct CallbackIdsHash { // same members. It is sufficient to just check the first CallbackId in the vectors. If // they match, they are the same. If they do not match, they are not the same. std::size_t operator()(const std::vector<CallbackId>& callbackIds) const { - return std::hash<CallbackId>{}((callbackIds.empty()) ? 0 : callbackIds.front()); + return std::hash<int64_t>{}((callbackIds.empty()) ? 0 : callbackIds.front().id); } }; diff --git a/libs/gui/include/gui/LayerState.h b/libs/gui/include/gui/LayerState.h index d2d1e5b91c..b4f62f2206 100644 --- a/libs/gui/include/gui/LayerState.h +++ b/libs/gui/include/gui/LayerState.h @@ -84,6 +84,7 @@ struct layer_state_t { eLayerStackChanged = 0x00000080, eReleaseBufferListenerChanged = 0x00000400, eShadowRadiusChanged = 0x00000800, + eLayerCreated = 0x00001000, /* was eDetachChildren, now available 0x00002000, */ eRelativeLayerChanged = 0x00004000, eReparent = 0x00008000, diff --git a/libs/gui/include/gui/SurfaceComposerClient.h b/libs/gui/include/gui/SurfaceComposerClient.h index fe6a30aa42..5bbd8e3d85 100644 --- a/libs/gui/include/gui/SurfaceComposerClient.h +++ b/libs/gui/include/gui/SurfaceComposerClient.h @@ -336,7 +336,7 @@ public: struct CallbackInfo { // All the callbacks that have been requested for a TransactionCompletedListener in the // Transaction - std::unordered_set<CallbackId> callbackIds; + std::unordered_set<CallbackId, CallbackIdHash> callbackIds; // All the SurfaceControls that have been modified in this TransactionCompletedListener's // process that require a callback if there is one or more callbackIds set. std::unordered_set<sp<SurfaceControl>, SCHash> surfaceControls; @@ -484,9 +484,15 @@ public: // Sets information about the priority of the frame. Transaction& setFrameRateSelectionPriority(const sp<SurfaceControl>& sc, int32_t priority); + Transaction& addTransactionCallback(TransactionCompletedCallbackTakesContext callback, + void* callbackContext, CallbackId::Type callbackType); + Transaction& addTransactionCompletedCallback( TransactionCompletedCallbackTakesContext callback, void* callbackContext); + Transaction& addTransactionCommittedCallback( + TransactionCompletedCallbackTakesContext callback, void* callbackContext); + // ONLY FOR BLAST ADAPTER Transaction& notifyProducerDisconnect(const sp<SurfaceControl>& sc); // Set the framenumber generated by the graphics producer to mimic BufferQueue behaviour. @@ -567,6 +573,8 @@ public: static status_t overrideHdrTypes(const sp<IBinder>& display, const std::vector<ui::Hdr>& hdrTypes); + static status_t onPullAtom(const int32_t atomId, std::string* outData, bool* success); + static void setDisplayProjection(const sp<IBinder>& token, ui::Rotation orientation, const Rect& layerStackRect, const Rect& displayRect); @@ -619,14 +627,13 @@ public: class TransactionCompletedListener : public BnTransactionCompletedListener { TransactionCompletedListener(); - CallbackId getNextIdLocked() REQUIRES(mMutex); + int64_t getNextIdLocked() REQUIRES(mMutex); std::mutex mMutex; bool mListening GUARDED_BY(mMutex) = false; - CallbackId mCallbackIdCounter GUARDED_BY(mMutex) = 1; - + int64_t mCallbackIdCounter GUARDED_BY(mMutex) = 1; struct CallbackTranslation { TransactionCompletedCallback callbackFunction; std::unordered_map<sp<IBinder>, sp<SurfaceControl>, SurfaceComposerClient::IBinderHash> @@ -644,7 +651,8 @@ class TransactionCompletedListener : public BnTransactionCompletedListener { SurfaceStatsCallback callback; }; - std::unordered_map<CallbackId, CallbackTranslation> mCallbacks GUARDED_BY(mMutex); + std::unordered_map<CallbackId, CallbackTranslation, CallbackIdHash> mCallbacks + GUARDED_BY(mMutex); std::multimap<sp<IBinder>, sp<JankDataListener>> mJankListeners GUARDED_BY(mMutex); std::unordered_map<uint64_t /* graphicsBufferId */, ReleaseBufferCallback> mReleaseBufferCallbacks GUARDED_BY(mMutex); @@ -660,10 +668,12 @@ public: CallbackId addCallbackFunction( const TransactionCompletedCallback& callbackFunction, const std::unordered_set<sp<SurfaceControl>, SurfaceComposerClient::SCHash>& - surfaceControls); + surfaceControls, + CallbackId::Type callbackType); - void addSurfaceControlToCallbacks(const sp<SurfaceControl>& surfaceControl, - const std::unordered_set<CallbackId>& callbackIds); + void addSurfaceControlToCallbacks( + const sp<SurfaceControl>& surfaceControl, + const std::unordered_set<CallbackId, CallbackIdHash>& callbackIds); /* * Adds a jank listener to be informed about SurfaceFlinger's jank classification for a specific diff --git a/libs/gui/tests/BLASTBufferQueue_test.cpp b/libs/gui/tests/BLASTBufferQueue_test.cpp index a44f44fbe6..5a5da97599 100644 --- a/libs/gui/tests/BLASTBufferQueue_test.cpp +++ b/libs/gui/tests/BLASTBufferQueue_test.cpp @@ -522,16 +522,146 @@ TEST_F(BLASTBufferQueueTest, SetCrop_ScalingModeScaleCrop) { adapter.waitForCallbacks(); // capture screen and verify that it is red ASSERT_EQ(NO_ERROR, captureDisplay(mCaptureArgs, mCaptureResults)); + ASSERT_NO_FATAL_FAILURE(checkScreenCapture(r, g, b, + {10, 10, (int32_t)bufferSideLength - 10, + (int32_t)bufferSideLength - 10})); + ASSERT_NO_FATAL_FAILURE( + checkScreenCapture(0, 0, 0, + {0, 0, (int32_t)bufferSideLength, (int32_t)bufferSideLength}, + /*border*/ 0, /*outsideRegion*/ true)); +} - Rect bounds; - bounds.left = finalCropSideLength / 2; - bounds.top = 0; - bounds.right = bounds.left + finalCropSideLength; - bounds.bottom = finalCropSideLength; +TEST_F(BLASTBufferQueueTest, ScaleCroppedBufferToBufferSize) { + // add black background + auto bg = mClient->createSurface(String8("BGTest"), 0, 0, PIXEL_FORMAT_RGBA_8888, + ISurfaceComposerClient::eFXSurfaceEffect); + ASSERT_NE(nullptr, bg.get()); + Transaction t; + t.setLayerStack(bg, 0) + .setCrop(bg, Rect(0, 0, mDisplayWidth, mDisplayHeight)) + .setColor(bg, half3{0, 0, 0}) + .setLayer(bg, 0) + .apply(); - ASSERT_NO_FATAL_FAILURE(checkScreenCapture(r, g, b, bounds)); - ASSERT_NO_FATAL_FAILURE( - checkScreenCapture(0, 0, 0, bounds, /*border*/ 0, /*outsideRegion*/ true)); + Rect windowSize(1000, 1000); + Rect bufferSize(windowSize); + Rect bufferCrop(200, 200, 700, 700); + + BLASTBufferQueueHelper adapter(mSurfaceControl, windowSize.getWidth(), windowSize.getHeight()); + sp<IGraphicBufferProducer> igbProducer; + setUpProducer(adapter, igbProducer); + int slot; + sp<Fence> fence; + sp<GraphicBuffer> buf; + auto ret = igbProducer->dequeueBuffer(&slot, &fence, bufferSize.getWidth(), + bufferSize.getHeight(), PIXEL_FORMAT_RGBA_8888, + GRALLOC_USAGE_SW_WRITE_OFTEN, nullptr, nullptr); + ASSERT_EQ(IGraphicBufferProducer::BUFFER_NEEDS_REALLOCATION, ret); + ASSERT_EQ(OK, igbProducer->requestBuffer(slot, &buf)); + + uint32_t* bufData; + buf->lock(static_cast<uint32_t>(GraphicBuffer::USAGE_SW_WRITE_OFTEN), + reinterpret_cast<void**>(&bufData)); + // fill buffer with grey + fillBuffer(bufData, bufferSize, buf->getStride(), 127, 127, 127); + + // fill crop area with different colors so we can verify the cropped region has been scaled + // correctly. + fillBuffer(bufData, Rect(200, 200, 450, 450), buf->getStride(), /* rgb */ 255, 0, 0); + fillBuffer(bufData, Rect(200, 451, 450, 700), buf->getStride(), /* rgb */ 0, 255, 0); + fillBuffer(bufData, Rect(451, 200, 700, 450), buf->getStride(), /* rgb */ 0, 0, 255); + fillBuffer(bufData, Rect(451, 451, 700, 700), buf->getStride(), /* rgb */ 255, 0, 0); + buf->unlock(); + + IGraphicBufferProducer::QueueBufferOutput qbOutput; + IGraphicBufferProducer::QueueBufferInput input(systemTime(), true /* autotimestamp */, + HAL_DATASPACE_UNKNOWN, + bufferCrop /* Rect::INVALID_RECT */, + NATIVE_WINDOW_SCALING_MODE_SCALE_TO_WINDOW, 0, + Fence::NO_FENCE); + igbProducer->queueBuffer(slot, input, &qbOutput); + ASSERT_NE(ui::Transform::ROT_INVALID, qbOutput.transformHint); + + adapter.waitForCallbacks(); + + ASSERT_EQ(NO_ERROR, captureDisplay(mCaptureArgs, mCaptureResults)); + + // Verify cropped region is scaled correctly. + ASSERT_NO_FATAL_FAILURE(checkScreenCapture(255, 0, 0, {10, 10, 490, 490})); + ASSERT_NO_FATAL_FAILURE(checkScreenCapture(0, 255, 0, {10, 510, 490, 990})); + ASSERT_NO_FATAL_FAILURE(checkScreenCapture(0, 0, 255, {510, 10, 990, 490})); + ASSERT_NO_FATAL_FAILURE(checkScreenCapture(255, 0, 0, {510, 510, 990, 990})); + // Verify outside region is black. + ASSERT_NO_FATAL_FAILURE(checkScreenCapture(0, 0, 0, + {0, 0, (int32_t)windowSize.getWidth(), + (int32_t)windowSize.getHeight()}, + /*border*/ 0, /*outsideRegion*/ true)); +} + +TEST_F(BLASTBufferQueueTest, ScaleCroppedBufferToWindowSize) { + // add black background + auto bg = mClient->createSurface(String8("BGTest"), 0, 0, PIXEL_FORMAT_RGBA_8888, + ISurfaceComposerClient::eFXSurfaceEffect); + ASSERT_NE(nullptr, bg.get()); + Transaction t; + t.setLayerStack(bg, 0) + .setCrop(bg, Rect(0, 0, mDisplayWidth, mDisplayHeight)) + .setColor(bg, half3{0, 0, 0}) + .setLayer(bg, 0) + .apply(); + + Rect windowSize(1000, 1000); + Rect bufferSize(500, 500); + Rect bufferCrop(100, 100, 350, 350); + + BLASTBufferQueueHelper adapter(mSurfaceControl, windowSize.getWidth(), windowSize.getHeight()); + sp<IGraphicBufferProducer> igbProducer; + setUpProducer(adapter, igbProducer); + int slot; + sp<Fence> fence; + sp<GraphicBuffer> buf; + auto ret = igbProducer->dequeueBuffer(&slot, &fence, bufferSize.getWidth(), + bufferSize.getHeight(), PIXEL_FORMAT_RGBA_8888, + GRALLOC_USAGE_SW_WRITE_OFTEN, nullptr, nullptr); + ASSERT_EQ(IGraphicBufferProducer::BUFFER_NEEDS_REALLOCATION, ret); + ASSERT_EQ(OK, igbProducer->requestBuffer(slot, &buf)); + + uint32_t* bufData; + buf->lock(static_cast<uint32_t>(GraphicBuffer::USAGE_SW_WRITE_OFTEN), + reinterpret_cast<void**>(&bufData)); + // fill buffer with grey + fillBuffer(bufData, bufferSize, buf->getStride(), 127, 127, 127); + + // fill crop area with different colors so we can verify the cropped region has been scaled + // correctly. + fillBuffer(bufData, Rect(100, 100, 225, 225), buf->getStride(), /* rgb */ 255, 0, 0); + fillBuffer(bufData, Rect(100, 226, 225, 350), buf->getStride(), /* rgb */ 0, 255, 0); + fillBuffer(bufData, Rect(226, 100, 350, 225), buf->getStride(), /* rgb */ 0, 0, 255); + fillBuffer(bufData, Rect(226, 226, 350, 350), buf->getStride(), /* rgb */ 255, 0, 0); + buf->unlock(); + + IGraphicBufferProducer::QueueBufferOutput qbOutput; + IGraphicBufferProducer::QueueBufferInput input(systemTime(), true /* autotimestamp */, + HAL_DATASPACE_UNKNOWN, + bufferCrop /* Rect::INVALID_RECT */, + NATIVE_WINDOW_SCALING_MODE_SCALE_TO_WINDOW, 0, + Fence::NO_FENCE); + igbProducer->queueBuffer(slot, input, &qbOutput); + ASSERT_NE(ui::Transform::ROT_INVALID, qbOutput.transformHint); + + adapter.waitForCallbacks(); + + ASSERT_EQ(NO_ERROR, captureDisplay(mCaptureArgs, mCaptureResults)); + // Verify cropped region is scaled correctly. + ASSERT_NO_FATAL_FAILURE(checkScreenCapture(255, 0, 0, {10, 10, 490, 490})); + ASSERT_NO_FATAL_FAILURE(checkScreenCapture(0, 255, 0, {10, 510, 490, 990})); + ASSERT_NO_FATAL_FAILURE(checkScreenCapture(0, 0, 255, {510, 10, 990, 490})); + ASSERT_NO_FATAL_FAILURE(checkScreenCapture(255, 0, 0, {510, 510, 990, 990})); + // Verify outside region is black. + ASSERT_NO_FATAL_FAILURE(checkScreenCapture(0, 0, 0, + {0, 0, (int32_t)windowSize.getWidth(), + (int32_t)windowSize.getHeight()}, + /*border*/ 0, /*outsideRegion*/ true)); } class TestProducerListener : public BnProducerListener { diff --git a/libs/gui/tests/Surface_test.cpp b/libs/gui/tests/Surface_test.cpp index 751b95af66..ea8c2957a1 100644 --- a/libs/gui/tests/Surface_test.cpp +++ b/libs/gui/tests/Surface_test.cpp @@ -102,14 +102,13 @@ protected: // test flakiness. mSurfaceControl = mComposerClient->createSurface( String8("Test Surface"), 32, 32, PIXEL_FORMAT_RGBA_8888, 0); + SurfaceComposerClient::Transaction().apply(true); ASSERT_TRUE(mSurfaceControl != nullptr); ASSERT_TRUE(mSurfaceControl->isValid()); Transaction t; - ASSERT_EQ(NO_ERROR, t.setLayer(mSurfaceControl, 0x7fffffff) - .show(mSurfaceControl) - .apply()); + ASSERT_EQ(NO_ERROR, t.setLayer(mSurfaceControl, 0x7fffffff).show(mSurfaceControl).apply()); mSurface = mSurfaceControl->getSurface(); ASSERT_TRUE(mSurface != nullptr); @@ -776,6 +775,10 @@ public: const std::vector<ui::Hdr>& /*hdrTypes*/) override { return NO_ERROR; } + status_t onPullAtom(const int32_t /*atomId*/, std::string* /*outData*/, + bool* /*success*/) override { + return NO_ERROR; + } status_t enableVSyncInjections(bool /*enable*/) override { return NO_ERROR; } diff --git a/libs/permission/Android.bp b/libs/permission/Android.bp index dd38224a60..a5712b319f 100644 --- a/libs/permission/Android.bp +++ b/libs/permission/Android.bp @@ -1,3 +1,12 @@ +package { + // See: http://go/android-license-faq + // A large-scale-change added 'default_applicable_licenses' to import + // all of the 'license_kinds' from "frameworks_native_license" + // to get the below license kinds: + // SPDX-license-identifier-Apache-2.0 + default_applicable_licenses: ["frameworks_native_license"], +} + cc_library_shared { name: "libpermission", srcs: [ diff --git a/libs/renderengine/include/renderengine/RenderEngine.h b/libs/renderengine/include/renderengine/RenderEngine.h index c8a0f0a034..ddaa7c7783 100644 --- a/libs/renderengine/include/renderengine/RenderEngine.h +++ b/libs/renderengine/include/renderengine/RenderEngine.h @@ -308,7 +308,8 @@ private: bool precacheToneMapperShaderOnly = false; bool supportsBackgroundBlur = false; RenderEngine::ContextPriority contextPriority = RenderEngine::ContextPriority::MEDIUM; - RenderEngine::RenderEngineType renderEngineType = RenderEngine::RenderEngineType::GLES; + RenderEngine::RenderEngineType renderEngineType = + RenderEngine::RenderEngineType::SKIA_GL_THREADED; }; } // namespace renderengine diff --git a/libs/renderengine/skia/AutoBackendTexture.cpp b/libs/renderengine/skia/AutoBackendTexture.cpp index c535597aea..9ed759f6bf 100644 --- a/libs/renderengine/skia/AutoBackendTexture.cpp +++ b/libs/renderengine/skia/AutoBackendTexture.cpp @@ -29,7 +29,8 @@ namespace renderengine { namespace skia { AutoBackendTexture::AutoBackendTexture(GrDirectContext* context, AHardwareBuffer* buffer, - bool isRender) { + bool isOutputBuffer) + : mIsOutputBuffer(isOutputBuffer) { ATRACE_CALL(); AHardwareBuffer_Desc desc; AHardwareBuffer_describe(buffer, &desc); @@ -40,8 +41,12 @@ AutoBackendTexture::AutoBackendTexture(GrDirectContext* context, AHardwareBuffer GrAHardwareBufferUtils::MakeBackendTexture(context, buffer, desc.width, desc.height, &mDeleteProc, &mUpdateProc, &mImageCtx, createProtectedImage, backendFormat, - isRender); + isOutputBuffer); mColorType = GrAHardwareBufferUtils::GetSkColorTypeFromBufferFormat(desc.format); + ALOGE_IF(!mBackendTexture.isValid(), + "Failed to create a valid texture. [%p]:[%d,%d] isProtected:%d isWriteable:%d " + "format:%d", + this, desc.width, desc.height, isOutputBuffer, createProtectedImage, desc.format); } void AutoBackendTexture::unref(bool releaseLocalResources) { @@ -92,13 +97,16 @@ sk_sp<SkImage> AutoBackendTexture::makeImage(ui::Dataspace dataspace, SkAlphaTyp mImage = image; mDataspace = dataspace; - LOG_ALWAYS_FATAL_IF(mImage == nullptr, "Unable to generate SkImage from buffer"); + LOG_ALWAYS_FATAL_IF(mImage == nullptr, + "Unable to generate SkImage. isTextureValid:%d dataspace:%d", + mBackendTexture.isValid(), dataspace); return mImage; } sk_sp<SkSurface> AutoBackendTexture::getOrCreateSurface(ui::Dataspace dataspace, GrDirectContext* context) { ATRACE_CALL(); + LOG_ALWAYS_FATAL_IF(!mIsOutputBuffer, "You can't generate a SkSurface for a read-only texture"); if (!mSurface.get() || mDataspace != dataspace) { sk_sp<SkSurface> surface = SkSurface::MakeFromBackendTexture(context, mBackendTexture, @@ -113,7 +121,9 @@ sk_sp<SkSurface> AutoBackendTexture::getOrCreateSurface(ui::Dataspace dataspace, } mDataspace = dataspace; - LOG_ALWAYS_FATAL_IF(mSurface == nullptr, "Unable to generate SkSurface"); + LOG_ALWAYS_FATAL_IF(mSurface == nullptr, + "Unable to generate SkSurface. isTextureValid:%d dataspace:%d", + mBackendTexture.isValid(), dataspace); return mSurface; } diff --git a/libs/renderengine/skia/AutoBackendTexture.h b/libs/renderengine/skia/AutoBackendTexture.h index 2d61cf854b..3133de60b5 100644 --- a/libs/renderengine/skia/AutoBackendTexture.h +++ b/libs/renderengine/skia/AutoBackendTexture.h @@ -41,34 +41,42 @@ public: // of shared ownership with Skia objects, so we wrap it here instead. class LocalRef { public: - LocalRef(AutoBackendTexture* texture) { setTexture(texture); } + LocalRef(GrDirectContext* context, AHardwareBuffer* buffer, bool isOutputBuffer) { + mTexture = new AutoBackendTexture(context, buffer, isOutputBuffer); + mTexture->ref(); + } ~LocalRef() { - // Destroying the texture is the same as setting it to null - setTexture(nullptr); + if (mTexture != nullptr) { + mTexture->unref(true); + } } - AutoBackendTexture* getTexture() const { return mTexture; } + // Makes a new SkImage from the texture content. + // As SkImages are immutable but buffer content is not, we create + // a new SkImage every time. + sk_sp<SkImage> makeImage(ui::Dataspace dataspace, SkAlphaType alphaType, + GrDirectContext* context) { + return mTexture->makeImage(dataspace, alphaType, context); + } + + // Makes a new SkSurface from the texture content, if needed. + sk_sp<SkSurface> getOrCreateSurface(ui::Dataspace dataspace, GrDirectContext* context) { + return mTexture->getOrCreateSurface(dataspace, context); + } DISALLOW_COPY_AND_ASSIGN(LocalRef); private: - // Sets the texture to locally ref-track. - void setTexture(AutoBackendTexture* texture) { - if (mTexture != nullptr) { - mTexture->unref(true); - } - - mTexture = texture; - if (mTexture != nullptr) { - mTexture->ref(); - } - } AutoBackendTexture* mTexture = nullptr; }; +private: // Creates a GrBackendTexture whose contents come from the provided buffer. - AutoBackendTexture(GrDirectContext* context, AHardwareBuffer* buffer, bool isRender); + AutoBackendTexture(GrDirectContext* context, AHardwareBuffer* buffer, bool isOutputBuffer); + + // The only way to invoke dtor is with unref, when mUsageCount is 0. + ~AutoBackendTexture() {} void ref() { mUsageCount++; } @@ -85,10 +93,6 @@ public: // Makes a new SkSurface from the texture content, if needed. sk_sp<SkSurface> getOrCreateSurface(ui::Dataspace dataspace, GrDirectContext* context); -private: - // The only way to invoke dtor is with unref, when mUsageCount is 0. - ~AutoBackendTexture() {} - GrBackendTexture mBackendTexture; GrAHardwareBufferUtils::DeleteImageProc mDeleteProc; GrAHardwareBufferUtils::UpdateImageProc mUpdateProc; @@ -99,6 +103,7 @@ private: int mUsageCount = 0; + const bool mIsOutputBuffer; sk_sp<SkImage> mImage = nullptr; sk_sp<SkSurface> mSurface = nullptr; ui::Dataspace mDataspace = ui::Dataspace::UNKNOWN; diff --git a/libs/renderengine/skia/SkiaGLRenderEngine.cpp b/libs/renderengine/skia/SkiaGLRenderEngine.cpp index 37d98a3a8f..0a84754d22 100644 --- a/libs/renderengine/skia/SkiaGLRenderEngine.cpp +++ b/libs/renderengine/skia/SkiaGLRenderEngine.cpp @@ -508,9 +508,9 @@ void SkiaGLRenderEngine::mapExternalTextureBuffer(const sp<GraphicBuffer>& buffe if (const auto& iter = cache.find(buffer->getId()); iter == cache.end()) { std::shared_ptr<AutoBackendTexture::LocalRef> imageTextureRef = - std::make_shared<AutoBackendTexture::LocalRef>( - new AutoBackendTexture(grContext.get(), buffer->toAHardwareBuffer(), - isRenderable)); + std::make_shared<AutoBackendTexture::LocalRef>(grContext.get(), + buffer->toAHardwareBuffer(), + isRenderable); cache.insert({buffer->getId(), imageTextureRef}); } // restore the original state of the protected context if necessary @@ -669,15 +669,17 @@ status_t SkiaGLRenderEngine::drawLayers(const DisplaySettings& display, if (const auto& it = cache.find(buffer->getBuffer()->getId()); it != cache.end()) { surfaceTextureRef = it->second; } else { - surfaceTextureRef = std::make_shared<AutoBackendTexture::LocalRef>( - new AutoBackendTexture(grContext.get(), buffer->getBuffer()->toAHardwareBuffer(), - true)); + surfaceTextureRef = + std::make_shared<AutoBackendTexture::LocalRef>(grContext.get(), + buffer->getBuffer() + ->toAHardwareBuffer(), + true); } const ui::Dataspace dstDataspace = mUseColorManagement ? display.outputDataspace : ui::Dataspace::UNKNOWN; sk_sp<SkSurface> dstSurface = - surfaceTextureRef->getTexture()->getOrCreateSurface(dstDataspace, grContext.get()); + surfaceTextureRef->getOrCreateSurface(dstDataspace, grContext.get()); SkCanvas* dstCanvas = mCapture->tryCapture(dstSurface.get()); if (dstCanvas == nullptr) { @@ -889,18 +891,17 @@ status_t SkiaGLRenderEngine::drawLayers(const DisplaySettings& display, // it. If we're using skia, we're guaranteed to run on a dedicated GPU thread so if // we didn't find anything in the cache then we intentionally did not cache this // buffer's resources. - imageTextureRef = std::make_shared<AutoBackendTexture::LocalRef>( - new AutoBackendTexture(grContext.get(), - item.buffer->getBuffer()->toAHardwareBuffer(), - false)); + imageTextureRef = std::make_shared< + AutoBackendTexture::LocalRef>(grContext.get(), + item.buffer->getBuffer()->toAHardwareBuffer(), + false); } sk_sp<SkImage> image = - imageTextureRef->getTexture()->makeImage(layerDataspace, - item.usePremultipliedAlpha - ? kPremul_SkAlphaType - : kUnpremul_SkAlphaType, - grContext.get()); + imageTextureRef->makeImage(layerDataspace, + item.usePremultipliedAlpha ? kPremul_SkAlphaType + : kUnpremul_SkAlphaType, + grContext.get()); auto texMatrix = getSkM44(item.textureTransform).asM33(); // textureTansform was intended to be passed directly into a shader, so when diff --git a/libs/renderengine/tests/RenderEngineTest.cpp b/libs/renderengine/tests/RenderEngineTest.cpp index d63c88bc02..34ef0a4f08 100644 --- a/libs/renderengine/tests/RenderEngineTest.cpp +++ b/libs/renderengine/tests/RenderEngineTest.cpp @@ -1402,7 +1402,8 @@ TEST_P(RenderEngineTest, drawLayers_fillBufferColorTransformZeroLayerAlpha_color fillBufferColorTransformZeroLayerAlpha<ColorSourceVariant>(); } -TEST_P(RenderEngineTest, drawLayers_fillBufferAndBlurBackground_colorSource) { +// TODO(b/186010146): reenable once swiftshader is happy with this test +TEST_P(RenderEngineTest, DISABLED_drawLayers_fillBufferAndBlurBackground_colorSource) { initializeRenderEngine(); fillBufferAndBlurBackground<ColorSourceVariant>(); } @@ -1477,7 +1478,8 @@ TEST_P(RenderEngineTest, drawLayers_fillBufferColorTransformZeroLayerAlpha_opaqu fillBufferColorTransformZeroLayerAlpha<BufferSourceVariant<ForceOpaqueBufferVariant>>(); } -TEST_P(RenderEngineTest, drawLayers_fillBufferAndBlurBackground_opaqueBufferSource) { +// TODO(b/186010146): reenable once swiftshader is happy with this test +TEST_P(RenderEngineTest, DISABLED_drawLayers_fillBufferAndBlurBackground_opaqueBufferSource) { initializeRenderEngine(); fillBufferAndBlurBackground<BufferSourceVariant<ForceOpaqueBufferVariant>>(); } @@ -1552,7 +1554,8 @@ TEST_P(RenderEngineTest, drawLayers_fillBufferColorTransformZeroLayerAlpha_buffe fillBufferColorTransformZeroLayerAlpha<BufferSourceVariant<RelaxOpaqueBufferVariant>>(); } -TEST_P(RenderEngineTest, drawLayers_fillBufferAndBlurBackground_bufferSource) { +// TODO(b/186010146): reenable once swiftshader is happy with this test +TEST_P(RenderEngineTest, DISABLED_drawLayers_fillBufferAndBlurBackground_bufferSource) { initializeRenderEngine(); fillBufferAndBlurBackground<BufferSourceVariant<RelaxOpaqueBufferVariant>>(); } diff --git a/libs/sensorprivacy/SensorPrivacyManager.cpp b/libs/sensorprivacy/SensorPrivacyManager.cpp index 47144696f2..b3537ce444 100644 --- a/libs/sensorprivacy/SensorPrivacyManager.cpp +++ b/libs/sensorprivacy/SensorPrivacyManager.cpp @@ -55,6 +55,22 @@ sp<hardware::ISensorPrivacyManager> SensorPrivacyManager::getService() return service; } +bool SensorPrivacyManager::supportsSensorToggle(int sensor) { + if (mSupportedCache.find(sensor) == mSupportedCache.end()) { + sp<hardware::ISensorPrivacyManager> service = getService(); + if (service != nullptr) { + bool result; + service->supportsSensorToggle(sensor, &result); + mSupportedCache[sensor] = result; + return result; + } + // if the SensorPrivacyManager is not available then assume sensor privacy feature isn't + // supported + return false; + } + return mSupportedCache[sensor]; +} + void SensorPrivacyManager::addSensorPrivacyListener( const sp<hardware::ISensorPrivacyListener>& listener) { diff --git a/libs/sensorprivacy/aidl/android/hardware/ISensorPrivacyManager.aidl b/libs/sensorprivacy/aidl/android/hardware/ISensorPrivacyManager.aidl index 629b8c2093..c8ceeb89af 100644 --- a/libs/sensorprivacy/aidl/android/hardware/ISensorPrivacyManager.aidl +++ b/libs/sensorprivacy/aidl/android/hardware/ISensorPrivacyManager.aidl @@ -20,6 +20,8 @@ import android.hardware.ISensorPrivacyListener; /** @hide */ interface ISensorPrivacyManager { + boolean supportsSensorToggle(int sensor); + void addSensorPrivacyListener(in ISensorPrivacyListener listener); void addIndividualSensorPrivacyListener(int userId, int sensor, in ISensorPrivacyListener listener); diff --git a/libs/sensorprivacy/include/sensorprivacy/SensorPrivacyManager.h b/libs/sensorprivacy/include/sensorprivacy/SensorPrivacyManager.h index 12778e16b6..fb4cbe9d55 100644 --- a/libs/sensorprivacy/include/sensorprivacy/SensorPrivacyManager.h +++ b/libs/sensorprivacy/include/sensorprivacy/SensorPrivacyManager.h @@ -22,6 +22,8 @@ #include <utils/threads.h> +#include <unordered_map> + // --------------------------------------------------------------------------- namespace android { @@ -35,6 +37,7 @@ public: SensorPrivacyManager(); + bool supportsSensorToggle(int sensor); void addSensorPrivacyListener(const sp<hardware::ISensorPrivacyListener>& listener); status_t addIndividualSensorPrivacyListener(int userId, int sensor, const sp<hardware::ISensorPrivacyListener>& listener); @@ -50,6 +53,8 @@ private: Mutex mLock; sp<hardware::ISensorPrivacyManager> mService; sp<hardware::ISensorPrivacyManager> getService(); + + std::unordered_map<int, bool> mSupportedCache; }; diff --git a/services/surfaceflinger/BufferStateLayer.cpp b/services/surfaceflinger/BufferStateLayer.cpp index ac2edbe717..7a5b20d78e 100644 --- a/services/surfaceflinger/BufferStateLayer.cpp +++ b/services/surfaceflinger/BufferStateLayer.cpp @@ -674,6 +674,11 @@ status_t BufferStateLayer::updateTexImage(bool& /*recomputeVisibleRegions*/, nse latchTime); } + std::deque<sp<CallbackHandle>> remainingHandles; + mFlinger->getTransactionCallbackInvoker() + .finalizeOnCommitCallbackHandles(mDrawingState.callbackHandles, remainingHandles); + mDrawingState.callbackHandles = remainingHandles; + mCurrentStateModified = false; return NO_ERROR; @@ -790,7 +795,7 @@ void BufferStateLayer::gatherBufferInfo() { mBufferInfo.mFence = s.acquireFence; mBufferInfo.mTransform = s.bufferTransform; mBufferInfo.mDataspace = translateDataspace(s.dataspace); - mBufferInfo.mCrop = computeCrop(s); + mBufferInfo.mCrop = computeBufferCrop(s); mBufferInfo.mScaleMode = NATIVE_WINDOW_SCALING_MODE_SCALE_TO_WINDOW; mBufferInfo.mSurfaceDamage = s.surfaceDamageRegion; mBufferInfo.mHdrMetadata = s.hdrMetadata; @@ -803,27 +808,11 @@ uint32_t BufferStateLayer::getEffectiveScalingMode() const { return NATIVE_WINDOW_SCALING_MODE_SCALE_TO_WINDOW; } -Rect BufferStateLayer::computeCrop(const State& s) { - if (s.crop.isEmpty() && s.buffer) { +Rect BufferStateLayer::computeBufferCrop(const State& s) { + if (s.buffer) { return s.buffer->getBuffer()->getBounds(); - } else if (s.buffer) { - Rect crop = s.crop; - crop.left = std::max(crop.left, 0); - crop.top = std::max(crop.top, 0); - uint32_t bufferWidth = s.buffer->getBuffer()->getWidth(); - uint32_t bufferHeight = s.buffer->getBuffer()->getHeight(); - if (bufferHeight <= std::numeric_limits<int32_t>::max() && - bufferWidth <= std::numeric_limits<int32_t>::max()) { - crop.right = std::min(crop.right, static_cast<int32_t>(bufferWidth)); - crop.bottom = std::min(crop.bottom, static_cast<int32_t>(bufferHeight)); - } - if (!crop.isValid()) { - // Crop rect is out of bounds, return whole buffer - return s.buffer->getBuffer()->getBounds(); - } - return crop; } - return s.crop; + return Rect::INVALID_RECT; } sp<Layer> BufferStateLayer::createClone() { diff --git a/services/surfaceflinger/BufferStateLayer.h b/services/surfaceflinger/BufferStateLayer.h index 4171092b58..af4fcae7ba 100644 --- a/services/surfaceflinger/BufferStateLayer.h +++ b/services/surfaceflinger/BufferStateLayer.h @@ -140,7 +140,7 @@ private: sp<Layer> createClone() override; // Crop that applies to the buffer - Rect computeCrop(const State& s); + Rect computeBufferCrop(const State& s); bool willPresentCurrentTransaction() const; diff --git a/services/surfaceflinger/CompositionEngine/include/compositionengine/CompositionRefreshArgs.h b/services/surfaceflinger/CompositionEngine/include/compositionengine/CompositionRefreshArgs.h index a0606b48f0..289cb119ca 100644 --- a/services/surfaceflinger/CompositionEngine/include/compositionengine/CompositionRefreshArgs.h +++ b/services/surfaceflinger/CompositionEngine/include/compositionengine/CompositionRefreshArgs.h @@ -79,6 +79,9 @@ struct CompositionRefreshArgs { // If set, causes the dirty regions to flash with the delay std::optional<std::chrono::microseconds> devOptFlashDirtyRegionsDelay; + + // The earliest time to send the present command to the HAL + std::chrono::steady_clock::time_point earliestPresentTime; }; } // namespace android::compositionengine diff --git a/services/surfaceflinger/CompositionEngine/include/compositionengine/LayerFE.h b/services/surfaceflinger/CompositionEngine/include/compositionengine/LayerFE.h index 1fd07b01f4..791e7db75c 100644 --- a/services/surfaceflinger/CompositionEngine/include/compositionengine/LayerFE.h +++ b/services/surfaceflinger/CompositionEngine/include/compositionengine/LayerFE.h @@ -138,6 +138,9 @@ public: // Gets the sequence number: a serial number that uniquely identifies a Layer virtual int32_t getSequence() const = 0; + + // Whether the layer should be rendered with rounded corners. + virtual bool hasRoundedCorners() const = 0; }; // TODO(b/121291683): Specialize std::hash<> for sp<T> so these and others can diff --git a/services/surfaceflinger/CompositionEngine/include/compositionengine/OutputLayer.h b/services/surfaceflinger/CompositionEngine/include/compositionengine/OutputLayer.h index 3a843273b9..ead941d1f9 100644 --- a/services/surfaceflinger/CompositionEngine/include/compositionengine/OutputLayer.h +++ b/services/surfaceflinger/CompositionEngine/include/compositionengine/OutputLayer.h @@ -16,6 +16,7 @@ #pragma once +#include <cstdint> #include <optional> #include <string> @@ -90,8 +91,13 @@ public: // Writes the geometry state to the HWC, or does nothing if this layer does // not use the HWC. If includeGeometry is false, the geometry state can be // skipped. If skipLayer is true, then the alpha of the layer is forced to - // 0 so that HWC will ignore it. - virtual void writeStateToHWC(bool includeGeometry, bool skipLayer) = 0; + // 0 so that HWC will ignore it. z specifies the order to draw the layer in + // (starting with 0 for the back layer, and increasing for each following + // layer). zIsOverridden specifies whether the layer has been reordered. + // isPeekingThrough specifies whether this layer will be shown through a + // hole punch in a layer above it. + virtual void writeStateToHWC(bool includeGeometry, bool skipLayer, uint32_t z, + bool zIsOverridden, bool isPeekingThrough) = 0; // Updates the cursor position with the HWC virtual void writeCursorPositionToHWC() const = 0; diff --git a/services/surfaceflinger/CompositionEngine/include/compositionengine/impl/OutputCompositionState.h b/services/surfaceflinger/CompositionEngine/include/compositionengine/impl/OutputCompositionState.h index 8f767d37f6..f0ef6d6e7d 100644 --- a/services/surfaceflinger/CompositionEngine/include/compositionengine/impl/OutputCompositionState.h +++ b/services/surfaceflinger/CompositionEngine/include/compositionengine/impl/OutputCompositionState.h @@ -115,6 +115,9 @@ struct OutputCompositionState { // Current target dataspace ui::Dataspace targetDataspace{ui::Dataspace::UNKNOWN}; + // The earliest time to send the present command to the HAL + std::chrono::steady_clock::time_point earliestPresentTime; + // Debugging void dump(std::string& result) const; }; diff --git a/services/surfaceflinger/CompositionEngine/include/compositionengine/impl/OutputLayer.h b/services/surfaceflinger/CompositionEngine/include/compositionengine/impl/OutputLayer.h index ae88e7839c..2488c66b6d 100644 --- a/services/surfaceflinger/CompositionEngine/include/compositionengine/impl/OutputLayer.h +++ b/services/surfaceflinger/CompositionEngine/include/compositionengine/impl/OutputLayer.h @@ -16,6 +16,7 @@ #pragma once +#include <cstdint> #include <memory> #include <string> @@ -42,7 +43,8 @@ public: void updateCompositionState(bool includeGeometry, bool forceClientComposition, ui::Transform::RotationFlags) override; - void writeStateToHWC(bool includeGeometry, bool skipLayer) override; + void writeStateToHWC(bool includeGeometry, bool skipLayer, uint32_t z, bool zIsOverridden, + bool isPeekingThrough) override; void writeCursorPositionToHWC() const override; HWC2::Layer* getHwcLayer() const override; @@ -66,7 +68,8 @@ protected: private: Rect calculateInitialCrop() const; - void writeOutputDependentGeometryStateToHWC(HWC2::Layer*, Hwc2::IComposerClient::Composition); + void writeOutputDependentGeometryStateToHWC(HWC2::Layer*, Hwc2::IComposerClient::Composition, + uint32_t z); void writeOutputIndependentGeometryStateToHWC(HWC2::Layer*, const LayerFECompositionState&, bool skipLayer); void writeOutputDependentPerFrameStateToHWC(HWC2::Layer*); @@ -74,7 +77,8 @@ private: void writeSolidColorStateToHWC(HWC2::Layer*, const LayerFECompositionState&); void writeSidebandStateToHWC(HWC2::Layer*, const LayerFECompositionState&); void writeBufferStateToHWC(HWC2::Layer*, const LayerFECompositionState&); - void writeCompositionTypeToHWC(HWC2::Layer*, Hwc2::IComposerClient::Composition); + void writeCompositionTypeToHWC(HWC2::Layer*, Hwc2::IComposerClient::Composition, + bool isPeekingThrough); void detectDisallowedCompositionTypeChange(Hwc2::IComposerClient::Composition from, Hwc2::IComposerClient::Composition to) const; }; diff --git a/services/surfaceflinger/CompositionEngine/include/compositionengine/impl/OutputLayerCompositionState.h b/services/surfaceflinger/CompositionEngine/include/compositionengine/impl/OutputLayerCompositionState.h index c61ec5991b..356965cf48 100644 --- a/services/surfaceflinger/CompositionEngine/include/compositionengine/impl/OutputLayerCompositionState.h +++ b/services/surfaceflinger/CompositionEngine/include/compositionengine/impl/OutputLayerCompositionState.h @@ -46,6 +46,10 @@ class Layer; class HWComposer; +namespace compositionengine { +class OutputLayer; +} // namespace compositionengine + namespace compositionengine::impl { // Note that fields that affect HW composer state may need to be mirrored into @@ -84,9 +88,6 @@ struct OutputLayerCompositionState { // The dataspace for this layer ui::Dataspace dataspace{ui::Dataspace::UNKNOWN}; - // The Z order index of this layer on this output - uint32_t z{0}; - // Overrides the buffer, acquire fence, and display frame stored in LayerFECompositionState struct { std::shared_ptr<renderengine::ExternalTexture> buffer = nullptr; @@ -96,6 +97,12 @@ struct OutputLayerCompositionState { ProjectionSpace displaySpace; Region damageRegion = Region::INVALID_REGION; Region visibleRegion; + + // The OutputLayer pointed to by this field will be rearranged to draw + // behind the OutputLayer represented by this CompositionState and will + // be visible through it. Unowned - the OutputLayer's lifetime will + // outlast this.) + OutputLayer* peekThroughLayer = nullptr; } overrideInfo; /* diff --git a/services/surfaceflinger/CompositionEngine/include/compositionengine/impl/planner/CachedSet.h b/services/surfaceflinger/CompositionEngine/include/compositionengine/impl/planner/CachedSet.h index 53f4a30fb8..1ee0e2f47c 100644 --- a/services/surfaceflinger/CompositionEngine/include/compositionengine/impl/planner/CachedSet.h +++ b/services/surfaceflinger/CompositionEngine/include/compositionengine/impl/planner/CachedSet.h @@ -105,12 +105,32 @@ public: void dump(std::string& result) const; + // Whether this represents a single layer with a buffer and rounded corners. + // If it is, we may be able to draw it by placing it behind another + // CachedSet and punching a hole. + bool requiresHolePunch() const; + + // Add a layer that will be drawn behind this one. ::render() will render a + // hole in this CachedSet's buffer, allowing the supplied layer to peek + // through. Must be called before ::render(). + // Will do nothing if this CachedSet is not opaque where the hole punch + // layer is displayed. + // If isFirstLayer is true, this CachedSet can be considered opaque because + // nothing (besides the hole punch layer) will be drawn behind it. + void addHolePunchLayerIfFeasible(const CachedSet&, bool isFirstLayer); + + // Retrieve the layer that will be drawn behind this one. + OutputLayer* getHolePunchLayer() const; + private: CachedSet() = default; const NonBufferHash mFingerprint; std::chrono::steady_clock::time_point mLastUpdate = std::chrono::steady_clock::now(); std::vector<Layer> mLayers; + + // Unowned. + const LayerState* mHolePunchLayer = nullptr; Rect mBounds = Rect::EMPTY_RECT; Region mVisibleRegion; size_t mAge = 0; diff --git a/services/surfaceflinger/CompositionEngine/include/compositionengine/impl/planner/Flattener.h b/services/surfaceflinger/CompositionEngine/include/compositionengine/impl/planner/Flattener.h index 2f2ad4c9b5..942592a9eb 100644 --- a/services/surfaceflinger/CompositionEngine/include/compositionengine/impl/planner/Flattener.h +++ b/services/surfaceflinger/CompositionEngine/include/compositionengine/impl/planner/Flattener.h @@ -36,7 +36,8 @@ class Predictor; class Flattener { public: - Flattener(Predictor& predictor) : mPredictor(predictor) {} + Flattener(Predictor& predictor, bool enableHolePunch = false) + : mEnableHolePunch(enableHolePunch), mPredictor(predictor) {} void setDisplaySize(ui::Size size) { mDisplaySize = size; } @@ -61,6 +62,7 @@ private: void buildCachedSets(std::chrono::steady_clock::time_point now); + const bool mEnableHolePunch; Predictor& mPredictor; ui::Size mDisplaySize; diff --git a/services/surfaceflinger/CompositionEngine/include/compositionengine/impl/planner/LayerState.h b/services/surfaceflinger/CompositionEngine/include/compositionengine/impl/planner/LayerState.h index f2f6c0b912..3391273679 100644 --- a/services/surfaceflinger/CompositionEngine/include/compositionengine/impl/planner/LayerState.h +++ b/services/surfaceflinger/CompositionEngine/include/compositionengine/impl/planner/LayerState.h @@ -53,20 +53,19 @@ enum class LayerStateField : uint32_t { Name = 1u << 1, DisplayFrame = 1u << 2, SourceCrop = 1u << 3, - ZOrder = 1u << 4, - BufferTransform = 1u << 5, - BlendMode = 1u << 6, - Alpha = 1u << 7, - LayerMetadata = 1u << 8, - VisibleRegion = 1u << 9, - Dataspace = 1u << 10, - PixelFormat = 1u << 11, - ColorTransform = 1u << 12, - SurfaceDamage = 1u << 13, - CompositionType = 1u << 14, - SidebandStream = 1u << 15, - Buffer = 1u << 16, - SolidColor = 1u << 17, + BufferTransform = 1u << 4, + BlendMode = 1u << 5, + Alpha = 1u << 6, + LayerMetadata = 1u << 7, + VisibleRegion = 1u << 8, + Dataspace = 1u << 9, + PixelFormat = 1u << 10, + ColorTransform = 1u << 11, + SurfaceDamage = 1u << 12, + CompositionType = 1u << 13, + SidebandStream = 1u << 14, + Buffer = 1u << 15, + SolidColor = 1u << 16, }; // clang-format on @@ -271,9 +270,6 @@ private: rect.top, rect.right, rect.bottom)}; }}; - OutputLayerState<uint32_t, LayerStateField::ZOrder> mZOrder{ - [](auto layer) { return layer->getState().z; }}; - using BufferTransformState = OutputLayerState<hardware::graphics::composer::hal::Transform, LayerStateField::BufferTransform>; BufferTransformState mBufferTransform{[](auto layer) { @@ -402,7 +398,7 @@ private: return std::vector<std::string>{stream.str()}; }}; - static const constexpr size_t kNumNonUniqueFields = 15; + static const constexpr size_t kNumNonUniqueFields = 14; std::array<StateInterface*, kNumNonUniqueFields> getNonUniqueFields() { std::array<const StateInterface*, kNumNonUniqueFields> constFields = @@ -417,10 +413,10 @@ private: std::array<const StateInterface*, kNumNonUniqueFields> getNonUniqueFields() const { return { - &mDisplayFrame, &mSourceCrop, &mZOrder, &mBufferTransform, - &mBlendMode, &mAlpha, &mLayerMetadata, &mVisibleRegion, - &mOutputDataspace, &mPixelFormat, &mColorTransform, &mCompositionType, - &mSidebandStream, &mBuffer, &mSolidColor, + &mDisplayFrame, &mSourceCrop, &mBufferTransform, &mBlendMode, + &mAlpha, &mLayerMetadata, &mVisibleRegion, &mOutputDataspace, + &mPixelFormat, &mColorTransform, &mCompositionType, &mSidebandStream, + &mBuffer, &mSolidColor, }; } }; diff --git a/services/surfaceflinger/CompositionEngine/include/compositionengine/impl/planner/Planner.h b/services/surfaceflinger/CompositionEngine/include/compositionengine/impl/planner/Planner.h index e6d2b636a3..c2037a899e 100644 --- a/services/surfaceflinger/CompositionEngine/include/compositionengine/impl/planner/Planner.h +++ b/services/surfaceflinger/CompositionEngine/include/compositionengine/impl/planner/Planner.h @@ -41,7 +41,7 @@ namespace compositionengine::impl::planner { // as a more efficient representation of parts of the layer stack. class Planner { public: - Planner() : mFlattener(mPredictor) {} + Planner(); void setDisplaySize(ui::Size); diff --git a/services/surfaceflinger/CompositionEngine/include/compositionengine/mock/LayerFE.h b/services/surfaceflinger/CompositionEngine/include/compositionengine/mock/LayerFE.h index dde8999524..d215bda891 100644 --- a/services/surfaceflinger/CompositionEngine/include/compositionengine/mock/LayerFE.h +++ b/services/surfaceflinger/CompositionEngine/include/compositionengine/mock/LayerFE.h @@ -43,6 +43,7 @@ public: MOCK_CONST_METHOD0(getDebugName, const char*()); MOCK_CONST_METHOD0(getSequence, int32_t()); + MOCK_CONST_METHOD0(hasRoundedCorners, bool()); }; } // namespace android::compositionengine::mock diff --git a/services/surfaceflinger/CompositionEngine/include/compositionengine/mock/OutputLayer.h b/services/surfaceflinger/CompositionEngine/include/compositionengine/mock/OutputLayer.h index 2454ff7188..358ed5a0b9 100644 --- a/services/surfaceflinger/CompositionEngine/include/compositionengine/mock/OutputLayer.h +++ b/services/surfaceflinger/CompositionEngine/include/compositionengine/mock/OutputLayer.h @@ -22,6 +22,7 @@ #include <compositionengine/OutputLayer.h> #include <compositionengine/impl/OutputLayerCompositionState.h> #include <gmock/gmock.h> +#include <cstdint> namespace android::compositionengine::mock { @@ -39,7 +40,7 @@ public: MOCK_METHOD0(editState, impl::OutputLayerCompositionState&()); MOCK_METHOD3(updateCompositionState, void(bool, bool, ui::Transform::RotationFlags)); - MOCK_METHOD2(writeStateToHWC, void(bool, bool)); + MOCK_METHOD5(writeStateToHWC, void(bool, bool, uint32_t, bool, bool)); MOCK_CONST_METHOD0(writeCursorPositionToHWC, void()); MOCK_CONST_METHOD0(getHwcLayer, HWC2::Layer*()); diff --git a/services/surfaceflinger/CompositionEngine/src/Display.cpp b/services/surfaceflinger/CompositionEngine/src/Display.cpp index a605fe1dc3..1ffb1c8e2d 100644 --- a/services/surfaceflinger/CompositionEngine/src/Display.cpp +++ b/services/surfaceflinger/CompositionEngine/src/Display.cpp @@ -367,6 +367,11 @@ compositionengine::Output::FrameFences Display::presentAndGetFrameFences() { return fences; } + { + ATRACE_NAME("wait for earliest present time"); + std::this_thread::sleep_until(getState().earliestPresentTime); + } + auto& hwc = getCompositionEngine().getHwComposer(); hwc.presentAndGetReleaseFences(*halDisplayIdOpt); diff --git a/services/surfaceflinger/CompositionEngine/src/Output.cpp b/services/surfaceflinger/CompositionEngine/src/Output.cpp index 3468b204fc..c809e1afad 100644 --- a/services/surfaceflinger/CompositionEngine/src/Output.cpp +++ b/services/surfaceflinger/CompositionEngine/src/Output.cpp @@ -455,12 +455,6 @@ void Output::collectVisibleLayers(const compositionengine::CompositionRefreshArg setReleasedLayers(refreshArgs); finalizePendingOutputLayers(); - - // Generate a simple Z-order values to each visible output layer - uint32_t zOrder = 0; - for (auto* outputLayer : getOutputLayersOrderedByZ()) { - outputLayer->editState().z = zOrder++; - } } void Output::ensureOutputLayerIfVisible(sp<compositionengine::LayerFE>& layerFE, @@ -711,20 +705,47 @@ void Output::writeCompositionState(const compositionengine::CompositionRefreshAr return; } + editState().earliestPresentTime = refreshArgs.earliestPresentTime; + + OutputLayer* peekThroughLayer = nullptr; sp<GraphicBuffer> previousOverride = nullptr; + bool includeGeometry = refreshArgs.updatingGeometryThisFrame; + uint32_t z = 0; + bool overrideZ = false; for (auto* layer : getOutputLayersOrderedByZ()) { + if (layer == peekThroughLayer) { + // No longer needed, although it should not show up again, so + // resetting it is not truly needed either. + peekThroughLayer = nullptr; + + // peekThroughLayer was already drawn ahead of its z order. + continue; + } bool skipLayer = false; - if (layer->getState().overrideInfo.buffer != nullptr) { - if (previousOverride != nullptr && - layer->getState().overrideInfo.buffer->getBuffer() == previousOverride) { + const auto& overrideInfo = layer->getState().overrideInfo; + if (overrideInfo.buffer != nullptr) { + if (previousOverride && overrideInfo.buffer->getBuffer() == previousOverride) { ALOGV("Skipping redundant buffer"); skipLayer = true; + } else { + // First layer with the override buffer. + if (overrideInfo.peekThroughLayer) { + peekThroughLayer = overrideInfo.peekThroughLayer; + + // Draw peekThroughLayer first. + overrideZ = true; + includeGeometry = true; + constexpr bool isPeekingThrough = true; + peekThroughLayer->writeStateToHWC(includeGeometry, false, z++, overrideZ, + isPeekingThrough); + } + + previousOverride = overrideInfo.buffer->getBuffer(); } - previousOverride = layer->getState().overrideInfo.buffer->getBuffer(); } - const bool includeGeometry = refreshArgs.updatingGeometryThisFrame; - layer->writeStateToHWC(includeGeometry, skipLayer); + constexpr bool isPeekingThrough = false; + layer->writeStateToHWC(includeGeometry, skipLayer, z++, overrideZ, isPeekingThrough); } } diff --git a/services/surfaceflinger/CompositionEngine/src/OutputLayer.cpp b/services/surfaceflinger/CompositionEngine/src/OutputLayer.cpp index 9ca8914deb..d3e2c253a9 100644 --- a/services/surfaceflinger/CompositionEngine/src/OutputLayer.cpp +++ b/services/surfaceflinger/CompositionEngine/src/OutputLayer.cpp @@ -21,6 +21,7 @@ #include <compositionengine/impl/OutputCompositionState.h> #include <compositionengine/impl/OutputLayer.h> #include <compositionengine/impl/OutputLayerCompositionState.h> +#include <cstdint> // TODO(b/129481165): remove the #pragma below and fix conversion issues #pragma clang diagnostic push @@ -312,7 +313,8 @@ void OutputLayer::updateCompositionState( } } -void OutputLayer::writeStateToHWC(bool includeGeometry, bool skipLayer) { +void OutputLayer::writeStateToHWC(bool includeGeometry, bool skipLayer, uint32_t z, + bool zIsOverridden, bool isPeekingThrough) { const auto& state = getState(); // Skip doing this if there is no HWC interface if (!state.hwc) { @@ -335,10 +337,11 @@ void OutputLayer::writeStateToHWC(bool includeGeometry, bool skipLayer) { // TODO(b/181172795): We now update geometry for all flattened layers. We should update it // only when the geometry actually changes - const bool isOverridden = state.overrideInfo.buffer != nullptr; + const bool isOverridden = + state.overrideInfo.buffer != nullptr || isPeekingThrough || zIsOverridden; const bool prevOverridden = state.hwc->stateOverridden; if (isOverridden || prevOverridden || skipLayer || includeGeometry) { - writeOutputDependentGeometryStateToHWC(hwcLayer.get(), requestedCompositionType); + writeOutputDependentGeometryStateToHWC(hwcLayer.get(), requestedCompositionType, z); writeOutputIndependentGeometryStateToHWC(hwcLayer.get(), *outputIndependentState, skipLayer); } @@ -346,7 +349,7 @@ void OutputLayer::writeStateToHWC(bool includeGeometry, bool skipLayer) { writeOutputDependentPerFrameStateToHWC(hwcLayer.get()); writeOutputIndependentPerFrameStateToHWC(hwcLayer.get(), *outputIndependentState); - writeCompositionTypeToHWC(hwcLayer.get(), requestedCompositionType); + writeCompositionTypeToHWC(hwcLayer.get(), requestedCompositionType, isPeekingThrough); // Always set the layer color after setting the composition type. writeSolidColorStateToHWC(hwcLayer.get(), *outputIndependentState); @@ -354,8 +357,9 @@ void OutputLayer::writeStateToHWC(bool includeGeometry, bool skipLayer) { editState().hwc->stateOverridden = isOverridden; } -void OutputLayer::writeOutputDependentGeometryStateToHWC( - HWC2::Layer* hwcLayer, hal::Composition requestedCompositionType) { +void OutputLayer::writeOutputDependentGeometryStateToHWC(HWC2::Layer* hwcLayer, + hal::Composition requestedCompositionType, + uint32_t z) { const auto& outputDependentState = getState(); Rect displayFrame = outputDependentState.displayFrame; @@ -382,9 +386,9 @@ void OutputLayer::writeOutputDependentGeometryStateToHWC( sourceCrop.bottom, to_string(error).c_str(), static_cast<int32_t>(error)); } - if (auto error = hwcLayer->setZOrder(outputDependentState.z); error != hal::Error::NONE) { - ALOGE("[%s] Failed to set Z %u: %s (%d)", getLayerFE().getDebugName(), - outputDependentState.z, to_string(error).c_str(), static_cast<int32_t>(error)); + if (auto error = hwcLayer->setZOrder(z); error != hal::Error::NONE) { + ALOGE("[%s] Failed to set Z %u: %s (%d)", getLayerFE().getDebugName(), z, + to_string(error).c_str(), static_cast<int32_t>(error)); } // Solid-color layers and overridden buffers should always use an identity transform. @@ -403,7 +407,10 @@ void OutputLayer::writeOutputDependentGeometryStateToHWC( void OutputLayer::writeOutputIndependentGeometryStateToHWC( HWC2::Layer* hwcLayer, const LayerFECompositionState& outputIndependentState, bool skipLayer) { - const auto blendMode = getState().overrideInfo.buffer + // If there is a peekThroughLayer, then this layer has a hole in it. We need to use + // PREMULTIPLIED so it will peek through. + const auto& overrideInfo = getState().overrideInfo; + const auto blendMode = overrideInfo.buffer || overrideInfo.peekThroughLayer ? hardware::graphics::composer::hal::BlendMode::PREMULTIPLIED : outputIndependentState.blendMode; if (auto error = hwcLayer->setBlendMode(blendMode); error != hal::Error::NONE) { @@ -558,11 +565,13 @@ void OutputLayer::writeBufferStateToHWC(HWC2::Layer* hwcLayer, } void OutputLayer::writeCompositionTypeToHWC(HWC2::Layer* hwcLayer, - hal::Composition requestedCompositionType) { + hal::Composition requestedCompositionType, + bool isPeekingThrough) { auto& outputDependentState = editState(); // If we are forcing client composition, we need to tell the HWC - if (outputDependentState.forceClientComposition) { + if (outputDependentState.forceClientComposition || + (!isPeekingThrough && getLayerFE().hasRoundedCorners())) { requestedCompositionType = hal::Composition::CLIENT; } diff --git a/services/surfaceflinger/CompositionEngine/src/OutputLayerCompositionState.cpp b/services/surfaceflinger/CompositionEngine/src/OutputLayerCompositionState.cpp index efd23dce98..b4c314c8d4 100644 --- a/services/surfaceflinger/CompositionEngine/src/OutputLayerCompositionState.cpp +++ b/services/surfaceflinger/CompositionEngine/src/OutputLayerCompositionState.cpp @@ -67,7 +67,6 @@ void OutputLayerCompositionState::dump(std::string& out) const { dumpVal(out, "sourceCrop", sourceCrop); dumpVal(out, "bufferTransform", toString(bufferTransform), bufferTransform); dumpVal(out, "dataspace", toString(dataspace), dataspace); - dumpVal(out, "z-index", z); dumpVal(out, "override buffer", overrideInfo.buffer.get()); dumpVal(out, "override acquire fence", overrideInfo.acquireFence.get()); dumpVal(out, "override display frame", overrideInfo.displayFrame); diff --git a/services/surfaceflinger/CompositionEngine/src/planner/CachedSet.cpp b/services/surfaceflinger/CompositionEngine/src/planner/CachedSet.cpp index 9955e29f4a..b31e89e247 100644 --- a/services/surfaceflinger/CompositionEngine/src/planner/CachedSet.cpp +++ b/services/surfaceflinger/CompositionEngine/src/planner/CachedSet.cpp @@ -193,6 +193,23 @@ void CachedSet::render(renderengine::RenderEngine& renderEngine, std::transform(layerSettings.cbegin(), layerSettings.cend(), std::back_inserter(layerSettingsPointers), [](const renderengine::LayerSettings& settings) { return &settings; }); + renderengine::LayerSettings holePunchSettings; + if (mHolePunchLayer) { + auto clientCompositionList = + mHolePunchLayer->getOutputLayer()->getLayerFE().prepareClientCompositionList( + targetSettings); + // Assume that the final layer contains the buffer that we want to + // replace with a hole punch. + holePunchSettings = clientCompositionList.back(); + LOG_ALWAYS_FATAL_IF(!holePunchSettings.source.buffer.buffer, "Expected to have a buffer!"); + // This mimics Layer::prepareClearClientComposition + holePunchSettings.source.buffer.buffer = nullptr; + holePunchSettings.source.solidColor = half3(0.0f, 0.0f, 0.0f); + holePunchSettings.disableBlending = true; + holePunchSettings.alpha = 0.0f; + holePunchSettings.name = std::string("hole punch layer"); + layerSettingsPointers.push_back(&holePunchSettings); + } if (sDebugHighlighLayers) { highlight = { @@ -241,6 +258,56 @@ void CachedSet::render(renderengine::RenderEngine& renderEngine, } } +bool CachedSet::requiresHolePunch() const { + // In order for the hole punch to be beneficial, the layer must be updating + // regularly, meaning it should not have been merged with other layers. + if (getLayerCount() != 1) { + return false; + } + + // There is no benefit to a hole punch unless the layer has a buffer. + if (!mLayers[0].getBuffer()) { + return false; + } + + const auto& layerFE = mLayers[0].getState()->getOutputLayer()->getLayerFE(); + if (layerFE.getCompositionState()->forceClientComposition) { + return false; + } + + return layerFE.hasRoundedCorners(); +} + +namespace { +bool contains(const Rect& outer, const Rect& inner) { + return outer.left <= inner.left && outer.right >= inner.right && outer.top <= inner.top && + outer.bottom >= inner.bottom; +} +}; // namespace + +void CachedSet::addHolePunchLayerIfFeasible(const CachedSet& holePunchLayer, bool isFirstLayer) { + // Verify that this CachedSet is opaque where the hole punch layer + // will draw. + const Rect& holePunchBounds = holePunchLayer.getBounds(); + for (const auto& layer : mLayers) { + // The first layer is considered opaque because nothing is behind it. + // Note that isOpaque is always false for a layer with rounded + // corners, even if the interior is opaque. In theory, such a layer + // could be used for a hole punch, but this is unlikely to happen in + // practice. + const auto* outputLayer = layer.getState()->getOutputLayer(); + if (contains(outputLayer->getState().displayFrame, holePunchBounds) && + (isFirstLayer || outputLayer->getLayerFE().getCompositionState()->isOpaque)) { + mHolePunchLayer = holePunchLayer.getFirstLayer().getState(); + return; + } + } +} + +OutputLayer* CachedSet::getHolePunchLayer() const { + return mHolePunchLayer ? mHolePunchLayer->getOutputLayer() : nullptr; +} + void CachedSet::dump(std::string& result) const { const auto now = std::chrono::steady_clock::now(); @@ -248,6 +315,11 @@ void CachedSet::dump(std::string& result) const { std::chrono::duration_cast<std::chrono::milliseconds>(now - mLastUpdate); base::StringAppendF(&result, " + Fingerprint %016zx, last update %sago, age %zd\n", mFingerprint, durationString(lastUpdate).c_str(), mAge); + { + const auto b = mTexture ? mTexture->getBuffer().get() : nullptr; + base::StringAppendF(&result, " Override buffer: %p\n", b); + } + base::StringAppendF(&result, " HolePunchLayer: %p\n", mHolePunchLayer); if (mLayers.size() == 1) { base::StringAppendF(&result, " Layer [%s]\n", mLayers[0].getName().c_str()); diff --git a/services/surfaceflinger/CompositionEngine/src/planner/Flattener.cpp b/services/surfaceflinger/CompositionEngine/src/planner/Flattener.cpp index 9c9649ccbe..13de622de0 100644 --- a/services/surfaceflinger/CompositionEngine/src/planner/Flattener.cpp +++ b/services/surfaceflinger/CompositionEngine/src/planner/Flattener.cpp @@ -209,6 +209,7 @@ bool Flattener::mergeWithCachedSets(const std::vector<const LayerState*>& layers ALOGV("[%s] Found ready buffer", __func__); size_t skipCount = mNewCachedSet->getLayerCount(); while (skipCount != 0) { + auto* peekThroughLayer = mNewCachedSet->getHolePunchLayer(); const size_t layerCount = currentLayerIter->getLayerCount(); for (size_t i = 0; i < layerCount; ++i) { OutputLayer::CompositionState& state = @@ -221,6 +222,7 @@ bool Flattener::mergeWithCachedSets(const std::vector<const LayerState*>& layers .displaySpace = mNewCachedSet->getOutputSpace(), .damageRegion = Region::INVALID_REGION, .visibleRegion = mNewCachedSet->getVisibleRegion(), + .peekThroughLayer = peekThroughLayer, }; ++incomingLayerIter; } @@ -244,6 +246,7 @@ bool Flattener::mergeWithCachedSets(const std::vector<const LayerState*>& layers // Skip the incoming layers corresponding to this valid current layer const size_t layerCount = currentLayerIter->getLayerCount(); + auto* peekThroughLayer = currentLayerIter->getHolePunchLayer(); for (size_t i = 0; i < layerCount; ++i) { OutputLayer::CompositionState& state = (*incomingLayerIter)->getOutputLayer()->editState(); @@ -255,6 +258,7 @@ bool Flattener::mergeWithCachedSets(const std::vector<const LayerState*>& layers .displaySpace = currentLayerIter->getOutputSpace(), .damageRegion = Region(), .visibleRegion = currentLayerIter->getVisibleRegion(), + .peekThroughLayer = peekThroughLayer, }; ++incomingLayerIter; } @@ -298,6 +302,11 @@ void Flattener::buildCachedSets(time_point now) { std::vector<Run> runs; bool isPartOfRun = false; + + // Keep track of the layer that follows a run. It's possible that we will + // render it with a hole-punch. + const CachedSet* holePunchLayer = nullptr; + for (auto currentSet = mLayers.cbegin(); currentSet != mLayers.cend(); ++currentSet) { if (now - currentSet->getLastUpdate() > kActiveLayerTimeout) { // Layer is inactive @@ -312,10 +321,20 @@ void Flattener::buildCachedSets(time_point now) { isPartOfRun = true; } } - } else { + } else if (isPartOfRun) { // Runs must be at least 2 sets long or there's nothing to combine - if (isPartOfRun && runs.back().start->getLayerCount() == runs.back().length) { + if (runs.back().start->getLayerCount() == runs.back().length) { runs.pop_back(); + } else { + // The prior run contained at least two sets. Currently, we'll + // only possibly merge a single run, so only keep track of a + // holePunchLayer if this is the first run. + if (runs.size() == 1) { + holePunchLayer = &(*currentSet); + } + + // TODO(b/185114532: Break out of the loop? We may find more runs, but we + // won't do anything with them. } isPartOfRun = false; @@ -341,6 +360,13 @@ void Flattener::buildCachedSets(time_point now) { mNewCachedSet->append(*currentSet); } + if (mEnableHolePunch && holePunchLayer && holePunchLayer->requiresHolePunch()) { + // Add the pip layer to mNewCachedSet, but in a special way - it should + // replace the buffer with a clear round rect. + mNewCachedSet->addHolePunchLayerIfFeasible(*holePunchLayer, + runs[0].start == mLayers.cbegin()); + } + // TODO(b/181192467): Actually compute new LayerState vector and corresponding hash for each run mPredictor.getPredictedPlan({}, 0); diff --git a/services/surfaceflinger/CompositionEngine/src/planner/LayerState.cpp b/services/surfaceflinger/CompositionEngine/src/planner/LayerState.cpp index ab8599796f..8423a124cf 100644 --- a/services/surfaceflinger/CompositionEngine/src/planner/LayerState.cpp +++ b/services/surfaceflinger/CompositionEngine/src/planner/LayerState.cpp @@ -155,10 +155,9 @@ std::optional<std::string> LayerState::compare(const LayerState& other) const { bool operator==(const LayerState& lhs, const LayerState& rhs) { return lhs.mId == rhs.mId && lhs.mName == rhs.mName && lhs.mDisplayFrame == rhs.mDisplayFrame && - lhs.mSourceCrop == rhs.mSourceCrop && lhs.mZOrder == rhs.mZOrder && - lhs.mBufferTransform == rhs.mBufferTransform && lhs.mBlendMode == rhs.mBlendMode && - lhs.mAlpha == rhs.mAlpha && lhs.mLayerMetadata == rhs.mLayerMetadata && - lhs.mVisibleRegion == rhs.mVisibleRegion && + lhs.mSourceCrop == rhs.mSourceCrop && lhs.mBufferTransform == rhs.mBufferTransform && + lhs.mBlendMode == rhs.mBlendMode && lhs.mAlpha == rhs.mAlpha && + lhs.mLayerMetadata == rhs.mLayerMetadata && lhs.mVisibleRegion == rhs.mVisibleRegion && lhs.mOutputDataspace == rhs.mOutputDataspace && lhs.mPixelFormat == rhs.mPixelFormat && lhs.mColorTransform == rhs.mColorTransform && lhs.mCompositionType == rhs.mCompositionType && diff --git a/services/surfaceflinger/CompositionEngine/src/planner/Planner.cpp b/services/surfaceflinger/CompositionEngine/src/planner/Planner.cpp index 3a2534b847..7d2bf06133 100644 --- a/services/surfaceflinger/CompositionEngine/src/planner/Planner.cpp +++ b/services/surfaceflinger/CompositionEngine/src/planner/Planner.cpp @@ -19,12 +19,17 @@ #undef LOG_TAG #define LOG_TAG "Planner" +#include <android-base/properties.h> #include <compositionengine/LayerFECompositionState.h> #include <compositionengine/impl/OutputLayerCompositionState.h> #include <compositionengine/impl/planner/Planner.h> namespace android::compositionengine::impl::planner { +Planner::Planner() + : mFlattener(mPredictor, + base::GetBoolProperty(std::string("debug.sf.enable_hole_punch_pip"), false)) {} + void Planner::setDisplaySize(ui::Size size) { mFlattener.setDisplaySize(size); } diff --git a/services/surfaceflinger/CompositionEngine/tests/OutputLayerTest.cpp b/services/surfaceflinger/CompositionEngine/tests/OutputLayerTest.cpp index 4c3f4940cc..5a92f26ff8 100644 --- a/services/surfaceflinger/CompositionEngine/tests/OutputLayerTest.cpp +++ b/services/surfaceflinger/CompositionEngine/tests/OutputLayerTest.cpp @@ -689,7 +689,6 @@ TEST_F(OutputLayerUpdateCompositionStateTest, clientCompositionForcedFromArgumen struct OutputLayerWriteStateToHWCTest : public OutputLayerTest { static constexpr hal::Error kError = hal::Error::UNSUPPORTED; static constexpr FloatRect kSourceCrop{11.f, 12.f, 13.f, 14.f}; - static constexpr uint32_t kZOrder = 21u; static constexpr Hwc2::Transform kBufferTransform = static_cast<Hwc2::Transform>(31); static constexpr Hwc2::Transform kOverrideBufferTransform = static_cast<Hwc2::Transform>(0); static constexpr Hwc2::IComposerClient::BlendMode kBlendMode = @@ -735,7 +734,6 @@ struct OutputLayerWriteStateToHWCTest : public OutputLayerTest { outputLayerState.displayFrame = kDisplayFrame; outputLayerState.sourceCrop = kSourceCrop; - outputLayerState.z = kZOrder; outputLayerState.bufferTransform = static_cast<Hwc2::Transform>(kBufferTransform); outputLayerState.outputSpaceVisibleRegion = kOutputSpaceVisibleRegion; outputLayerState.dataspace = kDataspace; @@ -785,7 +783,7 @@ struct OutputLayerWriteStateToHWCTest : public OutputLayerTest { float alpha = kAlpha) { EXPECT_CALL(*mHwcLayer, setDisplayFrame(displayFrame)).WillOnce(Return(kError)); EXPECT_CALL(*mHwcLayer, setSourceCrop(sourceCrop)).WillOnce(Return(kError)); - EXPECT_CALL(*mHwcLayer, setZOrder(kZOrder)).WillOnce(Return(kError)); + EXPECT_CALL(*mHwcLayer, setZOrder(_)).WillOnce(Return(kError)); EXPECT_CALL(*mHwcLayer, setTransform(bufferTransform)).WillOnce(Return(kError)); EXPECT_CALL(*mHwcLayer, setBlendMode(blendMode)).WillOnce(Return(kError)); @@ -878,19 +876,22 @@ const std::vector<uint8_t> OutputLayerWriteStateToHWCTest::kLayerGenericMetadata TEST_F(OutputLayerWriteStateToHWCTest, doesNothingIfNoFECompositionState) { EXPECT_CALL(*mLayerFE, getCompositionState()).WillOnce(Return(nullptr)); - mOutputLayer.writeStateToHWC(/*includeGeometry*/ true, /*skipLayer*/ false); + mOutputLayer.writeStateToHWC(/*includeGeometry*/ true, /*skipLayer*/ false, 0, + /*zIsOverridden*/ false, /*isPeekingThrough*/ false); } TEST_F(OutputLayerWriteStateToHWCTest, doesNothingIfNoHWCState) { mOutputLayer.editState().hwc.reset(); - mOutputLayer.writeStateToHWC(/*includeGeometry*/ true, /*skipLayer*/ false); + mOutputLayer.writeStateToHWC(/*includeGeometry*/ true, /*skipLayer*/ false, 0, + /*zIsOverridden*/ false, /*isPeekingThrough*/ false); } TEST_F(OutputLayerWriteStateToHWCTest, doesNothingIfNoHWCLayer) { mOutputLayer.editState().hwc = impl::OutputLayerCompositionState::Hwc(nullptr); - mOutputLayer.writeStateToHWC(/*includeGeometry*/ true, /*skipLayer*/ false); + mOutputLayer.writeStateToHWC(/*includeGeometry*/ true, /*skipLayer*/ false, 0, + /*zIsOverridden*/ false, /*isPeekingThrough*/ false); } TEST_F(OutputLayerWriteStateToHWCTest, canSetAllState) { @@ -898,8 +899,10 @@ TEST_F(OutputLayerWriteStateToHWCTest, canSetAllState) { expectPerFrameCommonCalls(); expectNoSetCompositionTypeCall(); + EXPECT_CALL(*mLayerFE, hasRoundedCorners()).WillOnce(Return(false)); - mOutputLayer.writeStateToHWC(/*includeGeometry*/ true, /*skipLayer*/ false); + mOutputLayer.writeStateToHWC(/*includeGeometry*/ true, /*skipLayer*/ false, 0, + /*zIsOverridden*/ false, /*isPeekingThrough*/ false); } TEST_F(OutputLayerTest, displayInstallOrientationBufferTransformSetTo90) { @@ -921,6 +924,7 @@ TEST_F(OutputLayerWriteStateToHWCTest, canSetPerFrameStateForSolidColor) { mLayerFEState.compositionType = Hwc2::IComposerClient::Composition::SOLID_COLOR; expectPerFrameCommonCalls(); + EXPECT_CALL(*mLayerFE, hasRoundedCorners()).WillOnce(Return(false)); // Setting the composition type should happen before setting the color. We // check this in this test only by setting up an testing::InSeqeuence @@ -929,7 +933,8 @@ TEST_F(OutputLayerWriteStateToHWCTest, canSetPerFrameStateForSolidColor) { expectSetCompositionTypeCall(Hwc2::IComposerClient::Composition::SOLID_COLOR); expectSetColorCall(); - mOutputLayer.writeStateToHWC(/*includeGeometry*/ false, /*skipLayer*/ false); + mOutputLayer.writeStateToHWC(/*includeGeometry*/ false, /*skipLayer*/ false, 0, + /*zIsOverridden*/ false, /*isPeekingThrough*/ false); } TEST_F(OutputLayerWriteStateToHWCTest, canSetPerFrameStateForSideband) { @@ -939,7 +944,10 @@ TEST_F(OutputLayerWriteStateToHWCTest, canSetPerFrameStateForSideband) { expectSetSidebandHandleCall(); expectSetCompositionTypeCall(Hwc2::IComposerClient::Composition::SIDEBAND); - mOutputLayer.writeStateToHWC(/*includeGeometry*/ false, /*skipLayer*/ false); + EXPECT_CALL(*mLayerFE, hasRoundedCorners()).WillOnce(Return(false)); + + mOutputLayer.writeStateToHWC(/*includeGeometry*/ false, /*skipLayer*/ false, 0, + /*zIsOverridden*/ false, /*isPeekingThrough*/ false); } TEST_F(OutputLayerWriteStateToHWCTest, canSetPerFrameStateForCursor) { @@ -949,7 +957,10 @@ TEST_F(OutputLayerWriteStateToHWCTest, canSetPerFrameStateForCursor) { expectSetHdrMetadataAndBufferCalls(); expectSetCompositionTypeCall(Hwc2::IComposerClient::Composition::CURSOR); - mOutputLayer.writeStateToHWC(/*includeGeometry*/ false, /*skipLayer*/ false); + EXPECT_CALL(*mLayerFE, hasRoundedCorners()).WillOnce(Return(false)); + + mOutputLayer.writeStateToHWC(/*includeGeometry*/ false, /*skipLayer*/ false, 0, + /*zIsOverridden*/ false, /*isPeekingThrough*/ false); } TEST_F(OutputLayerWriteStateToHWCTest, canSetPerFrameStateForDevice) { @@ -959,7 +970,10 @@ TEST_F(OutputLayerWriteStateToHWCTest, canSetPerFrameStateForDevice) { expectSetHdrMetadataAndBufferCalls(); expectSetCompositionTypeCall(Hwc2::IComposerClient::Composition::DEVICE); - mOutputLayer.writeStateToHWC(/*includeGeometry*/ false, /*skipLayer*/ false); + EXPECT_CALL(*mLayerFE, hasRoundedCorners()).WillOnce(Return(false)); + + mOutputLayer.writeStateToHWC(/*includeGeometry*/ false, /*skipLayer*/ false, 0, + /*zIsOverridden*/ false, /*isPeekingThrough*/ false); } TEST_F(OutputLayerWriteStateToHWCTest, compositionTypeIsNotSetIfUnchanged) { @@ -972,7 +986,10 @@ TEST_F(OutputLayerWriteStateToHWCTest, compositionTypeIsNotSetIfUnchanged) { expectSetColorCall(); expectNoSetCompositionTypeCall(); - mOutputLayer.writeStateToHWC(/*includeGeometry*/ false, /*skipLayer*/ false); + EXPECT_CALL(*mLayerFE, hasRoundedCorners()).WillOnce(Return(false)); + + mOutputLayer.writeStateToHWC(/*includeGeometry*/ false, /*skipLayer*/ false, 0, + /*zIsOverridden*/ false, /*isPeekingThrough*/ false); } TEST_F(OutputLayerWriteStateToHWCTest, compositionTypeIsSetToClientIfColorTransformNotSupported) { @@ -982,7 +999,8 @@ TEST_F(OutputLayerWriteStateToHWCTest, compositionTypeIsSetToClientIfColorTransf expectSetColorCall(); expectSetCompositionTypeCall(Hwc2::IComposerClient::Composition::CLIENT); - mOutputLayer.writeStateToHWC(/*includeGeometry*/ false, /*skipLayer*/ false); + mOutputLayer.writeStateToHWC(/*includeGeometry*/ false, /*skipLayer*/ false, 0, + /*zIsOverridden*/ false, /*isPeekingThrough*/ false); } TEST_F(OutputLayerWriteStateToHWCTest, compositionTypeIsSetToClientIfClientCompositionForced) { @@ -994,7 +1012,8 @@ TEST_F(OutputLayerWriteStateToHWCTest, compositionTypeIsSetToClientIfClientCompo expectSetColorCall(); expectSetCompositionTypeCall(Hwc2::IComposerClient::Composition::CLIENT); - mOutputLayer.writeStateToHWC(/*includeGeometry*/ false, /*skipLayer*/ false); + mOutputLayer.writeStateToHWC(/*includeGeometry*/ false, /*skipLayer*/ false, 0, + /*zIsOverridden*/ false, /*isPeekingThrough*/ false); } TEST_F(OutputLayerWriteStateToHWCTest, allStateIncludesMetadataIfPresent) { @@ -1007,7 +1026,10 @@ TEST_F(OutputLayerWriteStateToHWCTest, allStateIncludesMetadataIfPresent) { expectGenericLayerMetadataCalls(); expectSetCompositionTypeCall(Hwc2::IComposerClient::Composition::DEVICE); - mOutputLayer.writeStateToHWC(/*includeGeometry*/ true, /*skipLayer*/ false); + EXPECT_CALL(*mLayerFE, hasRoundedCorners()).WillOnce(Return(false)); + + mOutputLayer.writeStateToHWC(/*includeGeometry*/ true, /*skipLayer*/ false, 0, + /*zIsOverridden*/ false, /*isPeekingThrough*/ false); } TEST_F(OutputLayerWriteStateToHWCTest, perFrameStateDoesNotIncludeMetadataIfPresent) { @@ -1018,7 +1040,10 @@ TEST_F(OutputLayerWriteStateToHWCTest, perFrameStateDoesNotIncludeMetadataIfPres expectSetHdrMetadataAndBufferCalls(); expectSetCompositionTypeCall(Hwc2::IComposerClient::Composition::DEVICE); - mOutputLayer.writeStateToHWC(/*includeGeometry*/ false, /*skipLayer*/ false); + EXPECT_CALL(*mLayerFE, hasRoundedCorners()).WillOnce(Return(false)); + + mOutputLayer.writeStateToHWC(/*includeGeometry*/ false, /*skipLayer*/ false, 0, + /*zIsOverridden*/ false, /*isPeekingThrough*/ false); } TEST_F(OutputLayerWriteStateToHWCTest, includesOverrideInfoIfPresent) { @@ -1032,7 +1057,10 @@ TEST_F(OutputLayerWriteStateToHWCTest, includesOverrideInfoIfPresent) { expectSetHdrMetadataAndBufferCalls(kOverrideBuffer->getBuffer(), kOverrideFence); expectSetCompositionTypeCall(Hwc2::IComposerClient::Composition::DEVICE); - mOutputLayer.writeStateToHWC(/*includeGeometry*/ true, /*skipLayer*/ false); + EXPECT_CALL(*mLayerFE, hasRoundedCorners()).WillOnce(Return(false)); + + mOutputLayer.writeStateToHWC(/*includeGeometry*/ true, /*skipLayer*/ false, 0, + /*zIsOverridden*/ false, /*isPeekingThrough*/ false); } /* diff --git a/services/surfaceflinger/CompositionEngine/tests/OutputTest.cpp b/services/surfaceflinger/CompositionEngine/tests/OutputTest.cpp index e80100cc6e..7b71957021 100644 --- a/services/surfaceflinger/CompositionEngine/tests/OutputTest.cpp +++ b/services/surfaceflinger/CompositionEngine/tests/OutputTest.cpp @@ -30,6 +30,7 @@ #include <ui/Region.h> #include <cmath> +#include <cstdint> #include "CallOrderStateMachineHelper.h" #include "MockHWC2.h" @@ -783,15 +784,19 @@ TEST_F(OutputUpdateAndWriteCompositionStateTest, updatesLayerContentForAllLayers InjectedLayer layer2; InjectedLayer layer3; + uint32_t z = 0; EXPECT_CALL(*layer1.outputLayer, updateCompositionState(false, false, ui::Transform::ROT_180)); EXPECT_CALL(*layer1.outputLayer, - writeStateToHWC(/*includeGeometry*/ false, /*skipLayer*/ false)); + writeStateToHWC(/*includeGeometry*/ false, /*skipLayer*/ false, z++, + /*zIsOverridden*/ false, /*isPeekingThrough*/ false)); EXPECT_CALL(*layer2.outputLayer, updateCompositionState(false, false, ui::Transform::ROT_180)); EXPECT_CALL(*layer2.outputLayer, - writeStateToHWC(/*includeGeometry*/ false, /*skipLayer*/ false)); + writeStateToHWC(/*includeGeometry*/ false, /*skipLayer*/ false, z++, + /*zIsOverridden*/ false, /*isPeekingThrough*/ false)); EXPECT_CALL(*layer3.outputLayer, updateCompositionState(false, false, ui::Transform::ROT_180)); EXPECT_CALL(*layer3.outputLayer, - writeStateToHWC(/*includeGeometry*/ false, /*skipLayer*/ false)); + writeStateToHWC(/*includeGeometry*/ false, /*skipLayer*/ false, z++, + /*zIsOverridden*/ false, /*isPeekingThrough*/ false)); injectOutputLayer(layer1); injectOutputLayer(layer2); @@ -813,15 +818,19 @@ TEST_F(OutputUpdateAndWriteCompositionStateTest, updatesLayerGeometryAndContentF InjectedLayer layer2; InjectedLayer layer3; + uint32_t z = 0; EXPECT_CALL(*layer1.outputLayer, updateCompositionState(true, false, ui::Transform::ROT_0)); EXPECT_CALL(*layer1.outputLayer, - writeStateToHWC(/*includeGeometry*/ true, /*skipLayer*/ false)); + writeStateToHWC(/*includeGeometry*/ true, /*skipLayer*/ false, z++, + /*zIsOverridden*/ false, /*isPeekingThrough*/ false)); EXPECT_CALL(*layer2.outputLayer, updateCompositionState(true, false, ui::Transform::ROT_0)); EXPECT_CALL(*layer2.outputLayer, - writeStateToHWC(/*includeGeometry*/ true, /*skipLayer*/ false)); + writeStateToHWC(/*includeGeometry*/ true, /*skipLayer*/ false, z++, + /*zIsOverridden*/ false, /*isPeekingThrough*/ false)); EXPECT_CALL(*layer3.outputLayer, updateCompositionState(true, false, ui::Transform::ROT_0)); EXPECT_CALL(*layer3.outputLayer, - writeStateToHWC(/*includeGeometry*/ true, /*skipLayer*/ false)); + writeStateToHWC(/*includeGeometry*/ true, /*skipLayer*/ false, z++, + /*zIsOverridden*/ false, /*isPeekingThrough*/ false)); injectOutputLayer(layer1); injectOutputLayer(layer2); @@ -842,15 +851,19 @@ TEST_F(OutputUpdateAndWriteCompositionStateTest, forcesClientCompositionForAllLa InjectedLayer layer2; InjectedLayer layer3; + uint32_t z = 0; EXPECT_CALL(*layer1.outputLayer, updateCompositionState(false, true, ui::Transform::ROT_0)); EXPECT_CALL(*layer1.outputLayer, - writeStateToHWC(/*includeGeometry*/ false, /*skipLayer*/ false)); + writeStateToHWC(/*includeGeometry*/ false, /*skipLayer*/ false, z++, + /*zIsOverridden*/ false, /*isPeekingThrough*/ false)); EXPECT_CALL(*layer2.outputLayer, updateCompositionState(false, true, ui::Transform::ROT_0)); EXPECT_CALL(*layer2.outputLayer, - writeStateToHWC(/*includeGeometry*/ false, /*skipLayer*/ false)); + writeStateToHWC(/*includeGeometry*/ false, /*skipLayer*/ false, z++, + /*zIsOverridden*/ false, /*isPeekingThrough*/ false)); EXPECT_CALL(*layer3.outputLayer, updateCompositionState(false, true, ui::Transform::ROT_0)); EXPECT_CALL(*layer3.outputLayer, - writeStateToHWC(/*includeGeometry*/ false, /*skipLayer*/ false)); + writeStateToHWC(/*includeGeometry*/ false, /*skipLayer*/ false, z++, + /*zIsOverridden*/ false, /*isPeekingThrough*/ false)); injectOutputLayer(layer1); injectOutputLayer(layer2); @@ -1144,11 +1157,6 @@ TEST_F(OutputCollectVisibleLayersTest, processesCandidateLayersReversedAndSetsOu EXPECT_CALL(mOutput, finalizePendingOutputLayers()); mOutput.collectVisibleLayers(mRefreshArgs, mCoverageState); - - // Ensure all output layers have been assigned a simple/flattened z-order. - EXPECT_EQ(0u, mLayer1.outputLayerState.z); - EXPECT_EQ(1u, mLayer2.outputLayerState.z); - EXPECT_EQ(2u, mLayer3.outputLayerState.z); } /* @@ -3501,7 +3509,8 @@ struct OutputComposeSurfacesTest_SetsExpensiveRendering_ForBlur EXPECT_CALL(mLayer.outputLayer, updateCompositionState(false, true, ui::Transform::ROT_0)); EXPECT_CALL(mLayer.outputLayer, - writeStateToHWC(/*includeGeometry*/ false, /*skipLayer*/ false)); + writeStateToHWC(/*includeGeometry*/ false, /*skipLayer*/ false, 0, + /*zIsOverridden*/ false, /*isPeekingThrough*/ false)); EXPECT_CALL(mOutput, generateClientCompositionRequests(_, _, kDefaultOutputDataspace)) .WillOnce(Return(std::vector<LayerFE::LayerSettings>{})); EXPECT_CALL(mRenderEngine, drawLayers(_, _, _, false, _, _)).WillOnce(Return(NO_ERROR)); @@ -4080,16 +4089,20 @@ TEST_F(OutputUpdateAndWriteCompositionStateTest, handlesBackgroundBlurRequests) InjectedLayer layer2; InjectedLayer layer3; + uint32_t z = 0; // Layer requesting blur, or below, should request client composition. EXPECT_CALL(*layer1.outputLayer, updateCompositionState(false, true, ui::Transform::ROT_0)); EXPECT_CALL(*layer1.outputLayer, - writeStateToHWC(/*includeGeometry*/ false, /*skipLayer*/ false)); + writeStateToHWC(/*includeGeometry*/ false, /*skipLayer*/ false, z++, + /*zIsOverridden*/ false, /*isPeekingThrough*/ false)); EXPECT_CALL(*layer2.outputLayer, updateCompositionState(false, true, ui::Transform::ROT_0)); EXPECT_CALL(*layer2.outputLayer, - writeStateToHWC(/*includeGeometry*/ false, /*skipLayer*/ false)); + writeStateToHWC(/*includeGeometry*/ false, /*skipLayer*/ false, z++, + /*zIsOverridden*/ false, /*isPeekingThrough*/ false)); EXPECT_CALL(*layer3.outputLayer, updateCompositionState(false, false, ui::Transform::ROT_0)); EXPECT_CALL(*layer3.outputLayer, - writeStateToHWC(/*includeGeometry*/ false, /*skipLayer*/ false)); + writeStateToHWC(/*includeGeometry*/ false, /*skipLayer*/ false, z++, + /*zIsOverridden*/ false, /*isPeekingThrough*/ false)); layer2.layerFEState.backgroundBlurRadius = 10; @@ -4112,16 +4125,20 @@ TEST_F(OutputUpdateAndWriteCompositionStateTest, handlesBlurRegionRequests) { InjectedLayer layer2; InjectedLayer layer3; + uint32_t z = 0; // Layer requesting blur, or below, should request client composition. EXPECT_CALL(*layer1.outputLayer, updateCompositionState(false, true, ui::Transform::ROT_0)); EXPECT_CALL(*layer1.outputLayer, - writeStateToHWC(/*includeGeometry*/ false, /*skipLayer*/ false)); + writeStateToHWC(/*includeGeometry*/ false, /*skipLayer*/ false, z++, + /*zIsOverridden*/ false, /*isPeekingThrough*/ false)); EXPECT_CALL(*layer2.outputLayer, updateCompositionState(false, true, ui::Transform::ROT_0)); EXPECT_CALL(*layer2.outputLayer, - writeStateToHWC(/*includeGeometry*/ false, /*skipLayer*/ false)); + writeStateToHWC(/*includeGeometry*/ false, /*skipLayer*/ false, z++, + /*zIsOverridden*/ false, /*isPeekingThrough*/ false)); EXPECT_CALL(*layer3.outputLayer, updateCompositionState(false, false, ui::Transform::ROT_0)); EXPECT_CALL(*layer3.outputLayer, - writeStateToHWC(/*includeGeometry*/ false, /*skipLayer*/ false)); + writeStateToHWC(/*includeGeometry*/ false, /*skipLayer*/ false, z++, + /*zIsOverridden*/ false, /*isPeekingThrough*/ false)); BlurRegion region; layer2.layerFEState.blurRegions.push_back(region); diff --git a/services/surfaceflinger/CompositionEngine/tests/planner/LayerStateTest.cpp b/services/surfaceflinger/CompositionEngine/tests/planner/LayerStateTest.cpp index 83cc19b014..948c850735 100644 --- a/services/surfaceflinger/CompositionEngine/tests/planner/LayerStateTest.cpp +++ b/services/surfaceflinger/CompositionEngine/tests/planner/LayerStateTest.cpp @@ -41,8 +41,6 @@ const Rect sRectOne = Rect(10, 20, 30, 40); const Rect sRectTwo = Rect(40, 30, 20, 10); const FloatRect sFloatRectOne = FloatRect(100.f, 200.f, 300.f, 400.f); const FloatRect sFloatRectTwo = FloatRect(400.f, 300.f, 200.f, 100.f); -const constexpr int32_t sZOne = 100; -const constexpr int32_t sZTwo = 101; const constexpr float sAlphaOne = 0.25f; const constexpr float sAlphaTwo = 0.5f; const Region sRegionOne = Region(sRectOne); @@ -408,45 +406,6 @@ TEST_F(LayerStateTest, compareSourceCrop) { EXPECT_TRUE(otherLayerState->compare(*mLayerState)); } -TEST_F(LayerStateTest, updateZOrder) { - OutputLayerCompositionState outputLayerCompositionState; - outputLayerCompositionState.z = sZOne; - LayerFECompositionState layerFECompositionState; - setupMocksForLayer(mOutputLayer, mLayerFE, outputLayerCompositionState, - layerFECompositionState); - mLayerState = std::make_unique<LayerState>(&mOutputLayer); - - mock::OutputLayer newOutputLayer; - mock::LayerFE newLayerFE; - OutputLayerCompositionState outputLayerCompositionStateTwo; - outputLayerCompositionStateTwo.z = sZTwo; - setupMocksForLayer(newOutputLayer, newLayerFE, outputLayerCompositionStateTwo, - layerFECompositionState); - Flags<LayerStateField> updates = mLayerState->update(&newOutputLayer); - EXPECT_EQ(Flags<LayerStateField>(LayerStateField::ZOrder), updates); -} - -TEST_F(LayerStateTest, compareZOrder) { - OutputLayerCompositionState outputLayerCompositionState; - outputLayerCompositionState.z = sZOne; - LayerFECompositionState layerFECompositionState; - setupMocksForLayer(mOutputLayer, mLayerFE, outputLayerCompositionState, - layerFECompositionState); - mLayerState = std::make_unique<LayerState>(&mOutputLayer); - mock::OutputLayer newOutputLayer; - mock::LayerFE newLayerFE; - OutputLayerCompositionState outputLayerCompositionStateTwo; - outputLayerCompositionStateTwo.z = sZTwo; - setupMocksForLayer(newOutputLayer, newLayerFE, outputLayerCompositionStateTwo, - layerFECompositionState); - auto otherLayerState = std::make_unique<LayerState>(&newOutputLayer); - - verifyNonUniqueDifferingFields(*mLayerState, *otherLayerState, LayerStateField::ZOrder); - - EXPECT_TRUE(mLayerState->compare(*otherLayerState)); - EXPECT_TRUE(otherLayerState->compare(*mLayerState)); -} - TEST_F(LayerStateTest, updateBufferTransform) { OutputLayerCompositionState outputLayerCompositionState; outputLayerCompositionState.bufferTransform = Hwc2::Transform::FLIP_H; @@ -954,4 +913,4 @@ TEST_F(LayerStateTest, getNonBufferHash_filtersOutBuffers) { } } // namespace -} // namespace android::compositionengine::impl::planner
\ No newline at end of file +} // namespace android::compositionengine::impl::planner diff --git a/services/surfaceflinger/CompositionEngine/tests/planner/PredictorTest.cpp b/services/surfaceflinger/CompositionEngine/tests/planner/PredictorTest.cpp index 43e119fc11..1492707ad2 100644 --- a/services/surfaceflinger/CompositionEngine/tests/planner/PredictorTest.cpp +++ b/services/surfaceflinger/CompositionEngine/tests/planner/PredictorTest.cpp @@ -31,8 +31,6 @@ const FloatRect sFloatRectOne = FloatRect(100.f, 200.f, 300.f, 400.f); const FloatRect sFloatRectTwo = FloatRect(400.f, 300.f, 200.f, 100.f); const Rect sRectOne = Rect(1, 2, 3, 4); const Rect sRectTwo = Rect(4, 3, 2, 1); -const constexpr int32_t sZOne = 100; -const constexpr int32_t sZTwo = 101; const constexpr float sAlphaOne = 0.25f; const constexpr float sAlphaTwo = 0.5f; const Region sRegionOne = Region(sRectOne); @@ -194,11 +192,11 @@ TEST_F(LayerStackTest, getApproximateMatch_doesNotMatchManyDifferences) { .displayFrame = sRectOne, .sourceCrop = sFloatRectOne, .dataspace = ui::Dataspace::SRGB, - .z = sZOne, }; LayerFECompositionState layerFECompositionStateOne; layerFECompositionStateOne.alpha = sAlphaOne; layerFECompositionStateOne.colorTransformIsIdentity = true; + layerFECompositionStateOne.blendMode = hal::BlendMode::NONE; setupMocksForLayer(outputLayerOne, layerFEOne, outputLayerCompositionStateOne, layerFECompositionStateOne); LayerState layerStateOne(&outputLayerOne); @@ -210,12 +208,12 @@ TEST_F(LayerStackTest, getApproximateMatch_doesNotMatchManyDifferences) { .displayFrame = sRectTwo, .sourceCrop = sFloatRectTwo, .dataspace = ui::Dataspace::DISPLAY_P3, - .z = sZTwo, }; LayerFECompositionState layerFECompositionStateTwo; layerFECompositionStateTwo.alpha = sAlphaTwo; layerFECompositionStateTwo.colorTransformIsIdentity = false; layerFECompositionStateTwo.colorTransform = sMat4One; + layerFECompositionStateTwo.blendMode = hal::BlendMode::PREMULTIPLIED; setupMocksForLayer(outputLayerTwo, layerFETwo, outputLayerCompositionStateTwo, layerFECompositionStateTwo); LayerState layerStateTwo(&outputLayerTwo); @@ -264,7 +262,6 @@ TEST_F(LayerStackTest, getApproximateMatch_alwaysMatchesClientComposition) { .displayFrame = sRectOne, .sourceCrop = sFloatRectOne, .dataspace = ui::Dataspace::SRGB, - .z = sZOne, }; LayerFECompositionState layerFECompositionStateOne; layerFECompositionStateOne.buffer = new GraphicBuffer(); @@ -282,7 +279,6 @@ TEST_F(LayerStackTest, getApproximateMatch_alwaysMatchesClientComposition) { .displayFrame = sRectTwo, .sourceCrop = sFloatRectTwo, .dataspace = ui::Dataspace::DISPLAY_P3, - .z = sZTwo, }; LayerFECompositionState layerFECompositionStateTwo; layerFECompositionStateTwo.buffer = new GraphicBuffer(); @@ -346,6 +342,32 @@ struct PredictionTest : public testing::Test { } }; +TEST_F(LayerStackTest, reorderingChangesNonBufferHash) { + mock::OutputLayer outputLayerOne; + mock::LayerFE layerFEOne; + OutputLayerCompositionState outputLayerCompositionStateOne{ + .sourceCrop = sFloatRectOne, + }; + LayerFECompositionState layerFECompositionStateOne; + setupMocksForLayer(outputLayerOne, layerFEOne, outputLayerCompositionStateOne, + layerFECompositionStateOne); + LayerState layerStateOne(&outputLayerOne); + + mock::OutputLayer outputLayerTwo; + mock::LayerFE layerFETwo; + OutputLayerCompositionState outputLayerCompositionStateTwo{ + .sourceCrop = sFloatRectTwo, + }; + LayerFECompositionState layerFECompositionStateTwo; + setupMocksForLayer(outputLayerTwo, layerFETwo, outputLayerCompositionStateTwo, + layerFECompositionStateTwo); + LayerState layerStateTwo(&outputLayerTwo); + + NonBufferHash hash = getNonBufferHash({&layerStateOne, &layerStateTwo}); + NonBufferHash hashReverse = getNonBufferHash({&layerStateTwo, &layerStateOne}); + EXPECT_NE(hash, hashReverse); +} + TEST_F(PredictionTest, constructPrediction) { Plan plan; plan.addLayerType(hal::Composition::DEVICE); @@ -525,4 +547,4 @@ TEST_F(PredictorTest, recordMissedPlan_skipsApproximateMatch) { } } // namespace -} // namespace android::compositionengine::impl::planner
\ No newline at end of file +} // namespace android::compositionengine::impl::planner diff --git a/services/surfaceflinger/FrameTimeline/FrameTimeline.cpp b/services/surfaceflinger/FrameTimeline/FrameTimeline.cpp index 7468ac3534..be552c6435 100644 --- a/services/surfaceflinger/FrameTimeline/FrameTimeline.cpp +++ b/services/surfaceflinger/FrameTimeline/FrameTimeline.cpp @@ -653,8 +653,10 @@ void SurfaceFrame::traceActuals(int64_t displayFrameToken) const { // If prediction is expired, we can't use the predicted start time. Instead, just use a // start time a little earlier than the end time so that we have some info about this // frame in the trace. + nsecs_t endTime = + (mPresentState == PresentState::Dropped ? mDropTime : mActuals.endTime); packet->set_timestamp( - static_cast<uint64_t>(mActuals.endTime - kPredictionExpiredStartTimeDelta)); + static_cast<uint64_t>(endTime - kPredictionExpiredStartTimeDelta)); } else { packet->set_timestamp(static_cast<uint64_t>(mPredictions.startTime)); } diff --git a/services/surfaceflinger/Layer.cpp b/services/surfaceflinger/Layer.cpp index cf215adba2..4461420b46 100644 --- a/services/surfaceflinger/Layer.cpp +++ b/services/surfaceflinger/Layer.cpp @@ -531,7 +531,9 @@ void Layer::preparePerFrameCompositionState() { isOpaque(drawingState) && !usesRoundedCorners && getAlpha() == 1.0_hf; // Force client composition for special cases known only to the front-end. - if (isHdrY410() || usesRoundedCorners || drawShadows() || drawingState.blurRegions.size() > 0 || + // Rounded corners no longer force client composition, since we may use a + // hole punch so that the layer will appear to have rounded corners. + if (isHdrY410() || drawShadows() || drawingState.blurRegions.size() > 0 || compositionState->stretchEffect.hasEffect()) { compositionState->forceClientComposition = true; } diff --git a/services/surfaceflinger/Layer.h b/services/surfaceflinger/Layer.h index 9f3ea9ab39..688a2c3aff 100644 --- a/services/surfaceflinger/Layer.h +++ b/services/surfaceflinger/Layer.h @@ -600,6 +600,8 @@ public: // ignored. virtual RoundedCornerState getRoundedCornerState() const; + bool hasRoundedCorners() const override { return getRoundedCornerState().radius > .0f; } + virtual PixelFormat getPixelFormat() const { return PIXEL_FORMAT_NONE; } /** * Return whether this layer needs an input info. For most layer types diff --git a/services/surfaceflinger/Scheduler/OneShotTimer.cpp b/services/surfaceflinger/Scheduler/OneShotTimer.cpp index ce3b0c6cfa..d659398394 100644 --- a/services/surfaceflinger/Scheduler/OneShotTimer.cpp +++ b/services/surfaceflinger/Scheduler/OneShotTimer.cpp @@ -40,13 +40,21 @@ void calculateTimeoutTime(std::chrono::nanoseconds timestamp, timespec* spec) { namespace android { namespace scheduler { +std::chrono::steady_clock::time_point OneShotTimer::Clock::now() const { + return std::chrono::steady_clock::now(); +} + OneShotTimer::OneShotTimer(std::string name, const Interval& interval, const ResetCallback& resetCallback, - const TimeoutCallback& timeoutCallback) - : mName(std::move(name)), + const TimeoutCallback& timeoutCallback, + std::unique_ptr<OneShotTimer::Clock> clock) + : mClock(std::move(clock)), + mName(std::move(name)), mInterval(interval), mResetCallback(resetCallback), - mTimeoutCallback(timeoutCallback) {} + mTimeoutCallback(timeoutCallback) { + LOG_ALWAYS_FATAL_IF(!mClock, "Clock must not be provided"); +} OneShotTimer::~OneShotTimer() { stop(); @@ -112,7 +120,7 @@ void OneShotTimer::loop() { break; } - auto triggerTime = std::chrono::steady_clock::now() + mInterval; + auto triggerTime = mClock->now() + mInterval; state = TimerState::WAITING; while (state == TimerState::WAITING) { constexpr auto zero = std::chrono::steady_clock::duration::zero(); @@ -128,10 +136,9 @@ void OneShotTimer::loop() { state = checkForResetAndStop(state); if (state == TimerState::RESET) { - triggerTime = std::chrono::steady_clock::now() + mInterval; + triggerTime = mClock->now() + mInterval; state = TimerState::WAITING; - } else if (state == TimerState::WAITING && - (triggerTime - std::chrono::steady_clock::now()) <= zero) { + } else if (state == TimerState::WAITING && (triggerTime - mClock->now()) <= zero) { triggerTimeout = true; state = TimerState::IDLE; } diff --git a/services/surfaceflinger/Scheduler/OneShotTimer.h b/services/surfaceflinger/Scheduler/OneShotTimer.h index 3690ce7542..7285427abb 100644 --- a/services/surfaceflinger/Scheduler/OneShotTimer.h +++ b/services/surfaceflinger/Scheduler/OneShotTimer.h @@ -36,8 +36,17 @@ public: using ResetCallback = std::function<void()>; using TimeoutCallback = std::function<void()>; + class Clock { + public: + Clock() = default; + virtual ~Clock() = default; + + virtual std::chrono::steady_clock::time_point now() const; + }; + OneShotTimer(std::string name, const Interval& interval, const ResetCallback& resetCallback, - const TimeoutCallback& timeoutCallback); + const TimeoutCallback& timeoutCallback, + std::unique_ptr<OneShotTimer::Clock> = std::make_unique<OneShotTimer::Clock>()); ~OneShotTimer(); // Initializes and turns on the idle timer. @@ -78,6 +87,9 @@ private: // Thread waiting for timer to expire. std::thread mThread; + // Clock object for the timer. Mocked in unit tests. + std::unique_ptr<Clock> mClock; + // Semaphore to keep mThread synchronized. sem_t mSemaphore; diff --git a/services/surfaceflinger/Scheduler/Scheduler.cpp b/services/surfaceflinger/Scheduler/Scheduler.cpp index 1d25c7247f..57bd045945 100644 --- a/services/surfaceflinger/Scheduler/Scheduler.cpp +++ b/services/surfaceflinger/Scheduler/Scheduler.cpp @@ -927,4 +927,11 @@ void Scheduler::setPreferredRefreshRateForUid(FrameRateOverride frameRateOverrid } } +std::chrono::steady_clock::time_point Scheduler::getPreviousVsyncFrom( + nsecs_t expectedPresentTime) const { + const auto presentTime = std::chrono::nanoseconds(expectedPresentTime); + const auto vsyncPeriod = std::chrono::nanoseconds(mVsyncSchedule.tracker->currentPeriod()); + return std::chrono::steady_clock::time_point(presentTime - vsyncPeriod); +} + } // namespace android diff --git a/services/surfaceflinger/Scheduler/Scheduler.h b/services/surfaceflinger/Scheduler/Scheduler.h index 4d1f3c6572..49d3d93f36 100644 --- a/services/surfaceflinger/Scheduler/Scheduler.h +++ b/services/surfaceflinger/Scheduler/Scheduler.h @@ -149,6 +149,8 @@ public: bool isVsyncValid(nsecs_t expectedVsyncTimestamp, uid_t uid) const EXCLUDES(mFrameRateOverridesMutex); + std::chrono::steady_clock::time_point getPreviousVsyncFrom(nsecs_t expectedPresentTime) const; + void dump(std::string&) const; void dump(ConnectionHandle, std::string&) const; void dumpVsync(std::string&) const; diff --git a/services/surfaceflinger/SurfaceFlinger.cpp b/services/surfaceflinger/SurfaceFlinger.cpp index 5f5987f8a3..b04868225e 100644 --- a/services/surfaceflinger/SurfaceFlinger.cpp +++ b/services/surfaceflinger/SurfaceFlinger.cpp @@ -64,7 +64,6 @@ #include <private/android_filesystem_config.h> #include <private/gui/SyncFeatures.h> #include <renderengine/RenderEngine.h> -#include <statslog.h> #include <sys/types.h> #include <ui/ColorSpace.h> #include <ui/DebugUtils.h> @@ -627,7 +626,6 @@ void SurfaceFlinger::bootFinished() { ALOGI("Boot is finished (%ld ms)", long(ns2ms(duration)) ); mFrameTracer->initialize(); - mTimeStats->onBootFinished(); mFrameTimeline->onBootFinished(); // wait patiently for the window manager death @@ -1299,6 +1297,11 @@ status_t SurfaceFlinger::overrideHdrTypes(const sp<IBinder>& displayToken, return NO_ERROR; } +status_t SurfaceFlinger::onPullAtom(const int32_t atomId, std::string* pulledData, bool* success) { + *success = mTimeStats->onPullAtom(atomId, pulledData); + return NO_ERROR; +} + status_t SurfaceFlinger::getDisplayedContentSamplingAttributes(const sp<IBinder>& displayToken, ui::PixelFormat* outFormat, ui::Dataspace* outDataspace, @@ -1969,6 +1972,8 @@ void SurfaceFlinger::onMessageRefresh() { std::chrono::milliseconds(mDebugRegion > 1 ? mDebugRegion : 0); } + refreshArgs.earliestPresentTime = mScheduler->getPreviousVsyncFrom(mExpectedPresentTime); + mGeometryInvalid = false; // Store the present time just before calling to the composition engine so we could notify @@ -2029,6 +2034,9 @@ bool SurfaceFlinger::handleMessageInvalidate() { ATRACE_CALL(); bool refreshNeeded = handlePageFlip(); + // Send on commit callbacks + mTransactionCallbackInvoker.sendCallbacks(); + if (mVisibleRegionsDirty) { computeLayerBounds(); } @@ -2751,6 +2759,7 @@ void SurfaceFlinger::processDisplayChanged(const wp<IBinder>& displayToken, (currentState.orientedDisplaySpaceRect != drawingState.orientedDisplaySpaceRect)) { display->setProjection(currentState.orientation, currentState.layerStackSpaceRect, currentState.orientedDisplaySpaceRect); + mDefaultDisplayTransformHint = display->getTransformHint(); } if (currentState.width != drawingState.width || currentState.height != drawingState.height) { @@ -3257,70 +3266,38 @@ status_t SurfaceFlinger::addClientLayer(const sp<Client>& client, const sp<IBind const sp<IBinder>& parentHandle, const sp<Layer>& parentLayer, bool addToCurrentState, uint32_t* outTransformHint) { - // add this layer to the current state list - { - Mutex::Autolock _l(mStateLock); - sp<Layer> parent; - if (parentHandle != nullptr) { - parent = fromHandleLocked(parentHandle).promote(); - if (parent == nullptr) { - return NAME_NOT_FOUND; - } - } else { - parent = parentLayer; - } - - if (mNumLayers >= ISurfaceComposer::MAX_LAYERS) { - ALOGE("AddClientLayer failed, mNumLayers (%zu) >= MAX_LAYERS (%zu)", mNumLayers.load(), - ISurfaceComposer::MAX_LAYERS); - return NO_MEMORY; - } + if (mNumLayers >= ISurfaceComposer::MAX_LAYERS) { + ALOGE("AddClientLayer failed, mNumLayers (%zu) >= MAX_LAYERS (%zu)", mNumLayers.load(), + ISurfaceComposer::MAX_LAYERS); + return NO_MEMORY; + } - mLayersByLocalBinderToken.emplace(handle->localBinder(), lbc); + wp<IBinder> initialProducer; + if (gbc != nullptr) { + initialProducer = IInterface::asBinder(gbc); + } + setLayerCreatedState(handle, lbc, parentHandle, parentLayer, initialProducer); - if (parent == nullptr && addToCurrentState) { - mCurrentState.layersSortedByZ.add(lbc); - } else if (parent == nullptr) { - lbc->onRemovedFromCurrentState(); - } else if (parent->isRemovedFromCurrentState()) { - parent->addChild(lbc); - lbc->onRemovedFromCurrentState(); - } else { - parent->addChild(lbc); - } - - if (gbc != nullptr) { - mGraphicBufferProducerList.insert(IInterface::asBinder(gbc).get()); - LOG_ALWAYS_FATAL_IF(mGraphicBufferProducerList.size() > - mMaxGraphicBufferProducerListSize, - "Suspected IGBP leak: %zu IGBPs (%zu max), %zu Layers", - mGraphicBufferProducerList.size(), - mMaxGraphicBufferProducerListSize, mNumLayers.load()); - if (mGraphicBufferProducerList.size() > mGraphicBufferProducerListSizeLogThreshold) { - ALOGW("Suspected IGBP leak: %zu IGBPs (%zu max), %zu Layers", - mGraphicBufferProducerList.size(), mMaxGraphicBufferProducerListSize, - mNumLayers.load()); - } - } + // Create a transaction includes the initial parent and producer. + Vector<ComposerState> states; + Vector<DisplayState> displays; - if (const auto token = getInternalDisplayTokenLocked()) { - const ssize_t index = mCurrentState.displays.indexOfKey(token); - if (index >= 0) { - const DisplayDeviceState& state = mCurrentState.displays.valueAt(index); - lbc->updateTransformHint(ui::Transform::toRotationFlags(state.orientation)); - } - } - if (outTransformHint) { - *outTransformHint = lbc->getTransformHint(); - } + ComposerState composerState; + composerState.state.what = layer_state_t::eLayerCreated; + composerState.state.surface = handle; + states.add(composerState); - mLayersAdded = true; + lbc->updateTransformHint(mDefaultDisplayTransformHint); + if (outTransformHint) { + *outTransformHint = mDefaultDisplayTransformHint; } - // attach this layer to the client client->attachLayer(handle, lbc); - return NO_ERROR; + return setTransactionState(FrameTimelineInfo{}, states, displays, 0 /* flags */, nullptr, + InputWindowCommands{}, -1 /* desiredPresentTime */, + true /* isAutoTimestamp */, {}, false /* hasListenerCallbacks */, {}, + 0 /* Undefined transactionId */); } void SurfaceFlinger::removeGraphicBufferProducerAsync(const wp<IBinder>& binder) { @@ -3789,19 +3766,47 @@ bool SurfaceFlinger::callingThreadHasUnscopedSurfaceFlingerAccess(bool usePermis uint32_t SurfaceFlinger::setClientStateLocked( const FrameTimelineInfo& frameTimelineInfo, const ComposerState& composerState, int64_t desiredPresentTime, bool isAutoTimestamp, int64_t postTime, uint32_t permissions, - std::unordered_set<ListenerCallbacks, ListenerCallbacksHash>& listenerCallbacks) { + std::unordered_set<ListenerCallbacks, ListenerCallbacksHash>& outListenerCallbacks) { const layer_state_t& s = composerState.state; const bool privileged = permissions & Permission::ACCESS_SURFACE_FLINGER; + + std::vector<ListenerCallbacks> filteredListeners; for (auto& listener : s.listeners) { + // Starts a registration but separates the callback ids according to callback type. This + // allows the callback invoker to send on latch callbacks earlier. // note that startRegistration will not re-register if the listener has // already be registered for a prior surface control - mTransactionCallbackInvoker.startRegistration(listener); - listenerCallbacks.insert(listener); + + ListenerCallbacks onCommitCallbacks = listener.filter(CallbackId::Type::ON_COMMIT); + if (!onCommitCallbacks.callbackIds.empty()) { + mTransactionCallbackInvoker.startRegistration(onCommitCallbacks); + filteredListeners.push_back(onCommitCallbacks); + outListenerCallbacks.insert(onCommitCallbacks); + } + + ListenerCallbacks onCompleteCallbacks = listener.filter(CallbackId::Type::ON_COMPLETE); + if (!onCompleteCallbacks.callbackIds.empty()) { + mTransactionCallbackInvoker.startRegistration(onCompleteCallbacks); + filteredListeners.push_back(onCompleteCallbacks); + outListenerCallbacks.insert(onCompleteCallbacks); + } } + const uint64_t what = s.what; + uint32_t flags = 0; sp<Layer> layer = nullptr; if (s.surface) { - layer = fromHandleLocked(s.surface).promote(); + if (what & layer_state_t::eLayerCreated) { + layer = handleLayerCreatedLocked(s.surface, privileged); + if (layer) { + // put the created layer into mLayersByLocalBinderToken. + mLayersByLocalBinderToken.emplace(s.surface->localBinder(), layer); + flags |= eTransactionNeeded | eTraversalNeeded; + mLayersAdded = true; + } + } else { + layer = fromHandleLocked(s.surface).promote(); + } } else { // The client may provide us a null handle. Treat it as if the layer was removed. ALOGW("Attempt to set client state with a null layer handle"); @@ -3814,10 +3819,6 @@ uint32_t SurfaceFlinger::setClientStateLocked( return 0; } - uint32_t flags = 0; - - const uint64_t what = s.what; - // Only set by BLAST adapter layers if (what & layer_state_t::eProducerDisconnect) { layer->onDisconnect(); @@ -4049,8 +4050,8 @@ uint32_t SurfaceFlinger::setClientStateLocked( } } std::vector<sp<CallbackHandle>> callbackHandles; - if ((what & layer_state_t::eHasListenerCallbacksChanged) && (!s.listeners.empty())) { - for (auto& [listener, callbackIds] : s.listeners) { + if ((what & layer_state_t::eHasListenerCallbacksChanged) && (!filteredListeners.empty())) { + for (auto& [listener, callbackIds] : filteredListeners) { callbackHandles.emplace_back(new CallbackHandle(listener, callbackIds, s.surface)); } } @@ -4271,14 +4272,7 @@ status_t SurfaceFlinger::createBufferStateLayer(const sp<Client>& client, std::s sp<Layer>* outLayer) { LayerCreationArgs args(this, client, std::move(name), w, h, flags, std::move(metadata)); args.textureName = getNewTexture(); - sp<BufferStateLayer> layer; - { - // TODO (b/173538294): Investigate why we need mStateLock here and above in - // createBufferQueue layer. Is it the renderengine::Image? - Mutex::Autolock lock(mStateLock); - layer = getFactory().createBufferStateLayer(args); - - } + sp<BufferStateLayer> layer = getFactory().createBufferStateLayer(args); *handle = layer->getHandle(); *outLayer = layer; @@ -4366,7 +4360,7 @@ void SurfaceFlinger::onInitializeDisplays() { setPowerModeInternal(display, hal::PowerMode::ON); const nsecs_t vsyncPeriod = mRefreshRateConfigs->getCurrentRefreshRate().getVsyncPeriod(); mAnimFrameTracker.setDisplayRefreshPeriod(vsyncPeriod); - + mDefaultDisplayTransformHint = display->getTransformHint(); // Use phase of 0 since phase is not known. // Use latency of 0, which will snap to the ideal latency. DisplayStatInfo stats{0 /* vsyncTime */, vsyncPeriod}; @@ -5120,17 +5114,15 @@ status_t SurfaceFlinger::CheckTransactCodeCredentials(uint32_t code) { // captureLayers and captureDisplay will handle the permission check in the function case CAPTURE_LAYERS: case CAPTURE_DISPLAY: - case SET_DISPLAY_BRIGHTNESS: case SET_FRAME_TIMELINE_INFO: case GET_GPU_CONTEXT_PRIORITY: case GET_EXTRA_BUFFER_COUNT: { // This is not sensitive information, so should not require permission control. return OK; } + case SET_DISPLAY_BRIGHTNESS: case ADD_HDR_LAYER_INFO_LISTENER: case REMOVE_HDR_LAYER_INFO_LISTENER: { - // TODO (b/183985553): Should getting & setting brightness be part of this...? - // codes that require permission check IPCThreadState* ipc = IPCThreadState::self(); const int pid = ipc->getCallingPid(); const int uid = ipc->getCallingUid(); @@ -5165,6 +5157,13 @@ status_t SurfaceFlinger::CheckTransactCodeCredentials(uint32_t code) { } return PERMISSION_DENIED; } + case ON_PULL_ATOM: { + const int uid = IPCThreadState::self()->getCallingUid(); + if (uid == AID_SYSTEM) { + return OK; + } + return PERMISSION_DENIED; + } } // These codes are used for the IBinder protocol to either interrogate the recipient @@ -6622,6 +6621,87 @@ void SurfaceFlinger::TransactionState::traverseStatesWithBuffers( } } +void SurfaceFlinger::setLayerCreatedState(const sp<IBinder>& handle, const wp<Layer>& layer, + const wp<IBinder>& parent, const wp<Layer> parentLayer, + const wp<IBinder>& producer) { + Mutex::Autolock lock(mCreatedLayersLock); + mCreatedLayers[handle->localBinder()] = + std::make_unique<LayerCreatedState>(layer, parent, parentLayer, producer); +} + +auto SurfaceFlinger::getLayerCreatedState(const sp<IBinder>& handle) { + Mutex::Autolock lock(mCreatedLayersLock); + BBinder* b = nullptr; + if (handle) { + b = handle->localBinder(); + } + + if (b == nullptr) { + return std::unique_ptr<LayerCreatedState>(nullptr); + } + + auto it = mCreatedLayers.find(b); + if (it == mCreatedLayers.end()) { + ALOGE("Can't find layer from handle %p", handle.get()); + return std::unique_ptr<LayerCreatedState>(nullptr); + } + + auto state = std::move(it->second); + mCreatedLayers.erase(it); + return state; +} + +sp<Layer> SurfaceFlinger::handleLayerCreatedLocked(const sp<IBinder>& handle, bool privileged) { + const auto& state = getLayerCreatedState(handle); + if (!state) { + return nullptr; + } + + sp<Layer> layer = state->layer.promote(); + if (!layer) { + ALOGE("Invalid layer %p", state->layer.unsafe_get()); + return nullptr; + } + + sp<Layer> parent; + bool allowAddRoot = privileged; + if (state->initialParent != nullptr) { + parent = fromHandleLocked(state->initialParent.promote()).promote(); + if (parent == nullptr) { + ALOGE("Invalid parent %p", state->initialParent.unsafe_get()); + allowAddRoot = false; + } + } else if (state->initialParentLayer != nullptr) { + parent = state->initialParentLayer.promote(); + allowAddRoot = false; + } + + if (parent == nullptr && allowAddRoot) { + mCurrentState.layersSortedByZ.add(layer); + } else if (parent == nullptr) { + layer->onRemovedFromCurrentState(); + } else if (parent->isRemovedFromCurrentState()) { + parent->addChild(layer); + layer->onRemovedFromCurrentState(); + } else { + parent->addChild(layer); + } + + if (state->initialProducer != nullptr) { + mGraphicBufferProducerList.insert(state->initialProducer); + LOG_ALWAYS_FATAL_IF(mGraphicBufferProducerList.size() > mMaxGraphicBufferProducerListSize, + "Suspected IGBP leak: %zu IGBPs (%zu max), %zu Layers", + mGraphicBufferProducerList.size(), mMaxGraphicBufferProducerListSize, + mNumLayers.load()); + if (mGraphicBufferProducerList.size() > mGraphicBufferProducerListSizeLogThreshold) { + ALOGW("Suspected IGBP leak: %zu IGBPs (%zu max), %zu Layers", + mGraphicBufferProducerList.size(), mMaxGraphicBufferProducerListSize, + mNumLayers.load()); + } + } + + return layer; +} } // namespace android #if defined(__gl_h_) diff --git a/services/surfaceflinger/SurfaceFlinger.h b/services/surfaceflinger/SurfaceFlinger.h index 893b3d8751..cf1a545342 100644 --- a/services/surfaceflinger/SurfaceFlinger.h +++ b/services/surfaceflinger/SurfaceFlinger.h @@ -637,6 +637,7 @@ private: status_t getAnimationFrameStats(FrameStats* outStats) const override; status_t overrideHdrTypes(const sp<IBinder>& displayToken, const std::vector<ui::Hdr>& hdrTypes) override; + status_t onPullAtom(const int32_t atomId, std::string* pulledData, bool* success) override; status_t enableVSyncInjections(bool enable) override; status_t injectVSync(nsecs_t when) override; status_t getLayerDebugInfo(std::vector<LayerDebugInfo>* outLayers) override; @@ -1391,6 +1392,35 @@ private: std::unordered_map<DisplayId, sp<HdrLayerInfoReporter>> mHdrLayerInfoListeners GUARDED_BY(mStateLock); + mutable Mutex mCreatedLayersLock; + struct LayerCreatedState { + LayerCreatedState(const wp<Layer>& layer, const wp<IBinder>& parent, + const wp<Layer> parentLayer, const wp<IBinder>& producer) + : layer(layer), + initialParent(parent), + initialParentLayer(parentLayer), + initialProducer(producer) {} + wp<Layer> layer; + // Indicates the initial parent of the created layer, only used for creating layer in + // SurfaceFlinger. If nullptr, it may add the created layer into the current root layers. + wp<IBinder> initialParent; + wp<Layer> initialParentLayer; + // Indicates the initial graphic buffer producer of the created layer, only used for + // creating layer in SurfaceFlinger. + wp<IBinder> initialProducer; + }; + + // A temporay pool that store the created layers and will be added to current state in main + // thread. + std::unordered_map<BBinder*, std::unique_ptr<LayerCreatedState>> mCreatedLayers; + void setLayerCreatedState(const sp<IBinder>& handle, const wp<Layer>& layer, + const wp<IBinder>& parent, const wp<Layer> parentLayer, + const wp<IBinder>& producer); + auto getLayerCreatedState(const sp<IBinder>& handle); + sp<Layer> handleLayerCreatedLocked(const sp<IBinder>& handle, bool privileged) + REQUIRES(mStateLock); + + std::atomic<ui::Transform::RotationFlags> mDefaultDisplayTransformHint; }; } // namespace android diff --git a/services/surfaceflinger/TimeStats/Android.bp b/services/surfaceflinger/TimeStats/Android.bp index 62fddb4f47..bcc3e4e52a 100644 --- a/services/surfaceflinger/TimeStats/Android.bp +++ b/services/surfaceflinger/TimeStats/Android.bp @@ -18,20 +18,13 @@ cc_library { "libcutils", "liblog", "libprotobuf-cpp-lite", - "libprotoutil", - "libstatslog", - "libstatspull", - "libstatssocket", + "libtimestats_atoms_proto", "libtimestats_proto", "libui", "libutils", ], export_include_dirs: ["."], export_shared_lib_headers: [ - "libprotoutil", - "libstatslog", - "libstatspull", - "libstatssocket", "libtimestats_proto", ], cppflags: [ diff --git a/services/surfaceflinger/TimeStats/TimeStats.cpp b/services/surfaceflinger/TimeStats/TimeStats.cpp index 2094972ed0..3d82afa43a 100644 --- a/services/surfaceflinger/TimeStats/TimeStats.cpp +++ b/services/surfaceflinger/TimeStats/TimeStats.cpp @@ -19,11 +19,9 @@ #define LOG_TAG "TimeStats" #define ATRACE_TAG ATRACE_TAG_GRAPHICS -#include "TimeStats.h" - #include <android-base/stringprintf.h> -#include <android/util/ProtoOutputStream.h> #include <log/log.h> +#include <timestatsatomsproto/TimeStatsAtomsProtoHeader.h> #include <utils/String8.h> #include <utils/Timers.h> #include <utils/Trace.h> @@ -31,147 +29,102 @@ #include <algorithm> #include <chrono> +#include "TimeStats.h" #include "timestatsproto/TimeStatsHelper.h" namespace android { namespace impl { -AStatsManager_PullAtomCallbackReturn TimeStats::pullAtomCallback(int32_t atom_tag, - AStatsEventList* data, - void* cookie) { - impl::TimeStats* timeStats = reinterpret_cast<impl::TimeStats*>(cookie); - AStatsManager_PullAtomCallbackReturn result = AStatsManager_PULL_SKIP; - if (atom_tag == android::util::SURFACEFLINGER_STATS_GLOBAL_INFO) { - result = timeStats->populateGlobalAtom(data); - } else if (atom_tag == android::util::SURFACEFLINGER_STATS_LAYER_INFO) { - result = timeStats->populateLayerAtom(data); - } - - // Enable timestats now. The first full pull for a given build is expected to - // have empty or very little stats, as stats are first enabled after the - // first pull is completed for either the global or layer stats. - timeStats->enable(); - return result; -} - namespace { -// Histograms align with the order of fields in SurfaceflingerStatsLayerInfo. -const std::array<std::string, 6> kHistogramNames = { - "present2present", "post2present", "acquire2present", - "latch2present", "desired2present", "post2acquire", -}; - -std::string histogramToProtoByteString(const std::unordered_map<int32_t, int32_t>& histogram, - size_t maxPulledHistogramBuckets) { + +FrameTimingHistogram histogramToProto(const std::unordered_map<int32_t, int32_t>& histogram, + size_t maxPulledHistogramBuckets) { auto buckets = std::vector<std::pair<int32_t, int32_t>>(histogram.begin(), histogram.end()); std::sort(buckets.begin(), buckets.end(), [](std::pair<int32_t, int32_t>& left, std::pair<int32_t, int32_t>& right) { return left.second > right.second; }); - util::ProtoOutputStream proto; + FrameTimingHistogram histogramProto; int histogramSize = 0; for (const auto& bucket : buckets) { if (++histogramSize > maxPulledHistogramBuckets) { break; } - proto.write(android::util::FIELD_TYPE_INT32 | android::util::FIELD_COUNT_REPEATED | - 1 /* field id */, - (int32_t)bucket.first); - proto.write(android::util::FIELD_TYPE_INT64 | android::util::FIELD_COUNT_REPEATED | - 2 /* field id */, - (int64_t)bucket.second); + histogramProto.add_time_millis_buckets((int32_t)bucket.first); + histogramProto.add_frame_counts((int64_t)bucket.second); } - - std::string byteString; - proto.serializeToString(&byteString); - return byteString; + return histogramProto; } -std::string frameRateVoteToProtoByteString( - float refreshRate, - TimeStats::SetFrameRateVote::FrameRateCompatibility frameRateCompatibility, - TimeStats::SetFrameRateVote::Seamlessness seamlessness) { - util::ProtoOutputStream proto; - proto.write(android::util::FIELD_TYPE_FLOAT | 1 /* field id */, refreshRate); - proto.write(android::util::FIELD_TYPE_ENUM | 2 /* field id */, - static_cast<int>(frameRateCompatibility)); - proto.write(android::util::FIELD_TYPE_ENUM | 3 /* field id */, static_cast<int>(seamlessness)); - - std::string byteString; - proto.serializeToString(&byteString); - return byteString; +SurfaceflingerStatsLayerInfo_SetFrameRateVote frameRateVoteToProto( + const TimeStats::SetFrameRateVote& setFrameRateVote) { + using FrameRateCompatibilityEnum = + SurfaceflingerStatsLayerInfo::SetFrameRateVote::FrameRateCompatibility; + using SeamlessnessEnum = SurfaceflingerStatsLayerInfo::SetFrameRateVote::Seamlessness; + + SurfaceflingerStatsLayerInfo_SetFrameRateVote proto; + proto.set_frame_rate(setFrameRateVote.frameRate); + proto.set_frame_rate_compatibility( + static_cast<FrameRateCompatibilityEnum>(setFrameRateVote.frameRateCompatibility)); + proto.set_seamlessness(static_cast<SeamlessnessEnum>(setFrameRateVote.seamlessness)); + return proto; } } // namespace -AStatsManager_PullAtomCallbackReturn TimeStats::populateGlobalAtom(AStatsEventList* data) { +bool TimeStats::populateGlobalAtom(std::string* pulledData) { std::lock_guard<std::mutex> lock(mMutex); if (mTimeStats.statsStartLegacy == 0) { - return AStatsManager_PULL_SKIP; + return false; } flushPowerTimeLocked(); - + SurfaceflingerStatsGlobalInfoWrapper atomList; for (const auto& globalSlice : mTimeStats.stats) { - AStatsEvent* event = mStatsDelegate->addStatsEventToPullData(data); - mStatsDelegate->statsEventSetAtomId(event, android::util::SURFACEFLINGER_STATS_GLOBAL_INFO); - mStatsDelegate->statsEventWriteInt64(event, mTimeStats.totalFramesLegacy); - mStatsDelegate->statsEventWriteInt64(event, mTimeStats.missedFramesLegacy); - mStatsDelegate->statsEventWriteInt64(event, mTimeStats.clientCompositionFramesLegacy); - mStatsDelegate->statsEventWriteInt64(event, mTimeStats.displayOnTimeLegacy); - mStatsDelegate->statsEventWriteInt64(event, mTimeStats.presentToPresentLegacy.totalTime()); - mStatsDelegate->statsEventWriteInt32(event, mTimeStats.displayEventConnectionsCountLegacy); - std::string frameDurationBytes = - histogramToProtoByteString(mTimeStats.frameDurationLegacy.hist, - mMaxPulledHistogramBuckets); - mStatsDelegate->statsEventWriteByteArray(event, (const uint8_t*)frameDurationBytes.c_str(), - frameDurationBytes.size()); - std::string renderEngineTimingBytes = - histogramToProtoByteString(mTimeStats.renderEngineTimingLegacy.hist, - mMaxPulledHistogramBuckets); - mStatsDelegate->statsEventWriteByteArray(event, - (const uint8_t*)renderEngineTimingBytes.c_str(), - renderEngineTimingBytes.size()); - - mStatsDelegate->statsEventWriteInt32(event, globalSlice.second.jankPayload.totalFrames); - mStatsDelegate->statsEventWriteInt32(event, - globalSlice.second.jankPayload.totalJankyFrames); - mStatsDelegate->statsEventWriteInt32(event, globalSlice.second.jankPayload.totalSFLongCpu); - mStatsDelegate->statsEventWriteInt32(event, globalSlice.second.jankPayload.totalSFLongGpu); - mStatsDelegate->statsEventWriteInt32(event, - globalSlice.second.jankPayload.totalSFUnattributed); - mStatsDelegate->statsEventWriteInt32(event, - globalSlice.second.jankPayload.totalAppUnattributed); - mStatsDelegate->statsEventWriteInt32(event, - globalSlice.second.jankPayload.totalSFScheduling); - mStatsDelegate->statsEventWriteInt32(event, - globalSlice.second.jankPayload.totalSFPredictionError); - mStatsDelegate->statsEventWriteInt32(event, - globalSlice.second.jankPayload.totalAppBufferStuffing); - mStatsDelegate->statsEventWriteInt32(event, globalSlice.first.displayRefreshRateBucket); - std::string sfDeadlineMissedBytes = - histogramToProtoByteString(globalSlice.second.displayDeadlineDeltas.hist, - mMaxPulledHistogramBuckets); - mStatsDelegate->statsEventWriteByteArray(event, - (const uint8_t*)sfDeadlineMissedBytes.c_str(), - sfDeadlineMissedBytes.size()); - std::string sfPredictionErrorBytes = - histogramToProtoByteString(globalSlice.second.displayPresentDeltas.hist, - mMaxPulledHistogramBuckets); - mStatsDelegate->statsEventWriteByteArray(event, - (const uint8_t*)sfPredictionErrorBytes.c_str(), - sfPredictionErrorBytes.size()); - mStatsDelegate->statsEventWriteInt32(event, globalSlice.first.renderRateBucket); - mStatsDelegate->statsEventBuild(event); + SurfaceflingerStatsGlobalInfo* atom = atomList.add_atom(); + atom->set_total_frames(mTimeStats.totalFramesLegacy); + atom->set_missed_frames(mTimeStats.missedFramesLegacy); + atom->set_client_composition_frames(mTimeStats.clientCompositionFramesLegacy); + atom->set_display_on_millis(mTimeStats.displayOnTimeLegacy); + atom->set_animation_millis(mTimeStats.presentToPresentLegacy.totalTime()); + atom->set_event_connection_count(mTimeStats.displayEventConnectionsCountLegacy); + *atom->mutable_frame_duration() = + histogramToProto(mTimeStats.frameDurationLegacy.hist, mMaxPulledHistogramBuckets); + *atom->mutable_render_engine_timing() = + histogramToProto(mTimeStats.renderEngineTimingLegacy.hist, + mMaxPulledHistogramBuckets); + atom->set_total_timeline_frames(globalSlice.second.jankPayload.totalFrames); + atom->set_total_janky_frames(globalSlice.second.jankPayload.totalJankyFrames); + atom->set_total_janky_frames_with_long_cpu(globalSlice.second.jankPayload.totalSFLongCpu); + atom->set_total_janky_frames_with_long_gpu(globalSlice.second.jankPayload.totalSFLongGpu); + atom->set_total_janky_frames_sf_unattributed( + globalSlice.second.jankPayload.totalSFUnattributed); + atom->set_total_janky_frames_app_unattributed( + globalSlice.second.jankPayload.totalAppUnattributed); + atom->set_total_janky_frames_sf_scheduling( + globalSlice.second.jankPayload.totalSFScheduling); + atom->set_total_jank_frames_sf_prediction_error( + globalSlice.second.jankPayload.totalSFPredictionError); + atom->set_total_jank_frames_app_buffer_stuffing( + globalSlice.second.jankPayload.totalAppBufferStuffing); + atom->set_display_refresh_rate_bucket(globalSlice.first.displayRefreshRateBucket); + *atom->mutable_sf_deadline_misses() = + histogramToProto(globalSlice.second.displayDeadlineDeltas.hist, + mMaxPulledHistogramBuckets); + *atom->mutable_sf_prediction_errors() = + histogramToProto(globalSlice.second.displayPresentDeltas.hist, + mMaxPulledHistogramBuckets); + atom->set_render_rate_bucket(globalSlice.first.renderRateBucket); } + // Always clear data. clearGlobalLocked(); - return AStatsManager_PULL_SUCCESS; + return atomList.SerializeToString(pulledData); } -AStatsManager_PullAtomCallbackReturn TimeStats::populateLayerAtom(AStatsEventList* data) { +bool TimeStats::populateLayerAtom(std::string* pulledData) { std::lock_guard<std::mutex> lock(mMutex); std::vector<TimeStatsHelper::TimeStatsLayer*> dumpStats; @@ -198,69 +151,73 @@ AStatsManager_PullAtomCallbackReturn TimeStats::populateLayerAtom(AStatsEventLis dumpStats.resize(mMaxPulledLayers); } + SurfaceflingerStatsLayerInfoWrapper atomList; for (auto& layer : dumpStats) { - AStatsEvent* event = mStatsDelegate->addStatsEventToPullData(data); - mStatsDelegate->statsEventSetAtomId(event, android::util::SURFACEFLINGER_STATS_LAYER_INFO); - mStatsDelegate->statsEventWriteString8(event, layer->layerName.c_str()); - mStatsDelegate->statsEventWriteInt64(event, layer->totalFrames); - mStatsDelegate->statsEventWriteInt64(event, layer->droppedFrames); - - for (const auto& name : kHistogramNames) { - const auto& histogram = layer->deltas.find(name); - if (histogram == layer->deltas.cend()) { - mStatsDelegate->statsEventWriteByteArray(event, nullptr, 0); - } else { - std::string bytes = histogramToProtoByteString(histogram->second.hist, - mMaxPulledHistogramBuckets); - mStatsDelegate->statsEventWriteByteArray(event, (const uint8_t*)bytes.c_str(), - bytes.size()); - } + SurfaceflingerStatsLayerInfo* atom = atomList.add_atom(); + atom->set_layer_name(layer->layerName); + atom->set_total_frames(layer->totalFrames); + atom->set_dropped_frames(layer->droppedFrames); + const auto& present2PresentHist = layer->deltas.find("present2present"); + if (present2PresentHist != layer->deltas.cend()) { + *atom->mutable_present_to_present() = + histogramToProto(present2PresentHist->second.hist, mMaxPulledHistogramBuckets); + } + const auto& post2presentHist = layer->deltas.find("post2present"); + if (post2presentHist != layer->deltas.cend()) { + *atom->mutable_post_to_present() = + histogramToProto(post2presentHist->second.hist, mMaxPulledHistogramBuckets); + } + const auto& acquire2presentHist = layer->deltas.find("acquire2present"); + if (acquire2presentHist != layer->deltas.cend()) { + *atom->mutable_acquire_to_present() = + histogramToProto(acquire2presentHist->second.hist, mMaxPulledHistogramBuckets); + } + const auto& latch2presentHist = layer->deltas.find("latch2present"); + if (latch2presentHist != layer->deltas.cend()) { + *atom->mutable_latch_to_present() = + histogramToProto(latch2presentHist->second.hist, mMaxPulledHistogramBuckets); + } + const auto& desired2presentHist = layer->deltas.find("desired2present"); + if (desired2presentHist != layer->deltas.cend()) { + *atom->mutable_desired_to_present() = + histogramToProto(desired2presentHist->second.hist, mMaxPulledHistogramBuckets); + } + const auto& post2acquireHist = layer->deltas.find("post2acquire"); + if (post2acquireHist != layer->deltas.cend()) { + *atom->mutable_post_to_acquire() = + histogramToProto(post2acquireHist->second.hist, mMaxPulledHistogramBuckets); } - mStatsDelegate->statsEventWriteInt64(event, layer->lateAcquireFrames); - mStatsDelegate->statsEventWriteInt64(event, layer->badDesiredPresentFrames); - mStatsDelegate->statsEventWriteInt32(event, layer->uid); - mStatsDelegate->statsEventWriteInt32(event, layer->jankPayload.totalFrames); - mStatsDelegate->statsEventWriteInt32(event, layer->jankPayload.totalJankyFrames); - mStatsDelegate->statsEventWriteInt32(event, layer->jankPayload.totalSFLongCpu); - mStatsDelegate->statsEventWriteInt32(event, layer->jankPayload.totalSFLongGpu); - mStatsDelegate->statsEventWriteInt32(event, layer->jankPayload.totalSFUnattributed); - mStatsDelegate->statsEventWriteInt32(event, layer->jankPayload.totalAppUnattributed); - mStatsDelegate->statsEventWriteInt32(event, layer->jankPayload.totalSFScheduling); - mStatsDelegate->statsEventWriteInt32(event, layer->jankPayload.totalSFPredictionError); - mStatsDelegate->statsEventWriteInt32(event, layer->jankPayload.totalAppBufferStuffing); - mStatsDelegate->statsEventWriteInt32( - event, layer->displayRefreshRateBucket); // display_refresh_rate_bucket - mStatsDelegate->statsEventWriteInt32(event, layer->renderRateBucket); // render_rate_bucket - std::string frameRateVoteBytes = - frameRateVoteToProtoByteString(layer->setFrameRateVote.frameRate, - layer->setFrameRateVote.frameRateCompatibility, - layer->setFrameRateVote.seamlessness); - mStatsDelegate->statsEventWriteByteArray(event, (const uint8_t*)frameRateVoteBytes.c_str(), - frameRateVoteBytes.size()); // set_frame_rate_vote - std::string appDeadlineMissedBytes = - histogramToProtoByteString(layer->deltas["appDeadlineDeltas"].hist, - mMaxPulledHistogramBuckets); - mStatsDelegate->statsEventWriteByteArray(event, - (const uint8_t*)appDeadlineMissedBytes.c_str(), - appDeadlineMissedBytes.size()); - - mStatsDelegate->statsEventBuild(event); + atom->set_late_acquire_frames(layer->lateAcquireFrames); + atom->set_bad_desired_present_frames(layer->badDesiredPresentFrames); + atom->set_uid(layer->uid); + atom->set_total_timeline_frames(layer->jankPayload.totalFrames); + atom->set_total_janky_frames(layer->jankPayload.totalJankyFrames); + atom->set_total_janky_frames_with_long_cpu(layer->jankPayload.totalSFLongCpu); + atom->set_total_janky_frames_with_long_gpu(layer->jankPayload.totalSFLongGpu); + atom->set_total_janky_frames_sf_unattributed(layer->jankPayload.totalSFUnattributed); + atom->set_total_janky_frames_app_unattributed(layer->jankPayload.totalAppUnattributed); + atom->set_total_janky_frames_sf_scheduling(layer->jankPayload.totalSFScheduling); + atom->set_total_jank_frames_sf_prediction_error(layer->jankPayload.totalSFPredictionError); + atom->set_total_jank_frames_app_buffer_stuffing(layer->jankPayload.totalAppBufferStuffing); + atom->set_display_refresh_rate_bucket(layer->displayRefreshRateBucket); + atom->set_render_rate_bucket(layer->renderRateBucket); + *atom->mutable_set_frame_rate_vote() = frameRateVoteToProto(layer->setFrameRateVote); + *atom->mutable_app_deadline_misses() = + histogramToProto(layer->deltas["appDeadlineDeltas"].hist, + mMaxPulledHistogramBuckets); } + + // Always clear data. clearLayersLocked(); - return AStatsManager_PULL_SUCCESS; + return atomList.SerializeToString(pulledData); } -TimeStats::TimeStats() : TimeStats(nullptr, std::nullopt, std::nullopt) {} +TimeStats::TimeStats() : TimeStats(std::nullopt, std::nullopt) {} -TimeStats::TimeStats(std::unique_ptr<StatsEventDelegate> statsDelegate, - std::optional<size_t> maxPulledLayers, +TimeStats::TimeStats(std::optional<size_t> maxPulledLayers, std::optional<size_t> maxPulledHistogramBuckets) { - if (statsDelegate != nullptr) { - mStatsDelegate = std::move(statsDelegate); - } - if (maxPulledLayers) { mMaxPulledLayers = *maxPulledLayers; } @@ -270,18 +227,19 @@ TimeStats::TimeStats(std::unique_ptr<StatsEventDelegate> statsDelegate, } } -TimeStats::~TimeStats() { - std::lock_guard<std::mutex> lock(mMutex); - mStatsDelegate->clearStatsPullAtomCallback(android::util::SURFACEFLINGER_STATS_GLOBAL_INFO); - mStatsDelegate->clearStatsPullAtomCallback(android::util::SURFACEFLINGER_STATS_LAYER_INFO); -} +bool TimeStats::onPullAtom(const int atomId, std::string* pulledData) { + bool success = false; + if (atomId == 10062) { // SURFACEFLINGER_STATS_GLOBAL_INFO + success = populateGlobalAtom(pulledData); + } else if (atomId == 10063) { // SURFACEFLINGER_STATS_LAYER_INFO + success = populateLayerAtom(pulledData); + } -void TimeStats::onBootFinished() { - std::lock_guard<std::mutex> lock(mMutex); - mStatsDelegate->setStatsPullAtomCallback(android::util::SURFACEFLINGER_STATS_GLOBAL_INFO, - nullptr, TimeStats::pullAtomCallback, this); - mStatsDelegate->setStatsPullAtomCallback(android::util::SURFACEFLINGER_STATS_LAYER_INFO, - nullptr, TimeStats::pullAtomCallback, this); + // Enable timestats now. The first full pull for a given build is expected to + // have empty or very little stats, as stats are first enabled after the + // first pull is completed for either the global or layer stats. + enable(); + return success; } void TimeStats::parseArgs(bool asProto, const Vector<String16>& args, std::string& result) { @@ -787,7 +745,7 @@ void TimeStats::setPresentFence(int32_t layerId, uint64_t frameNumber, static const constexpr int32_t kValidJankyReason = JankType::DisplayHAL | JankType::SurfaceFlingerCpuDeadlineMissed | JankType::SurfaceFlingerGpuDeadlineMissed | JankType::AppDeadlineMissed | JankType::PredictionError | - JankType::SurfaceFlingerScheduling | JankType::BufferStuffing; + JankType::SurfaceFlingerScheduling; template <class T> static void updateJankPayload(T& t, int32_t reasons) { @@ -813,9 +771,11 @@ static void updateJankPayload(T& t, int32_t reasons) { if ((reasons & JankType::SurfaceFlingerScheduling) != 0) { t.jankPayload.totalSFScheduling++; } - if ((reasons & JankType::BufferStuffing) != 0) { - t.jankPayload.totalAppBufferStuffing++; - } + } + + // We want to track BufferStuffing separately as it can provide info on latency issues + if (reasons & JankType::BufferStuffing) { + t.jankPayload.totalAppBufferStuffing++; } } diff --git a/services/surfaceflinger/TimeStats/TimeStats.h b/services/surfaceflinger/TimeStats/TimeStats.h index a87b7cb4a6..5b0f5bd13d 100644 --- a/services/surfaceflinger/TimeStats/TimeStats.h +++ b/services/surfaceflinger/TimeStats/TimeStats.h @@ -29,9 +29,6 @@ #include <../Fps.h> #include <gui/JankInfo.h> -#include <stats_event.h> -#include <stats_pull_atom_callback.h> -#include <statslog.h> #include <timestatsproto/TimeStatsHelper.h> #include <timestatsproto/TimeStatsProtoHeader.h> #include <ui/FenceTime.h> @@ -54,9 +51,8 @@ public: virtual ~TimeStats() = default; - // Called once boot has been finished to perform additional capabilities, - // e.g. registration to statsd. - virtual void onBootFinished() = 0; + // Process a pull request from statsd. + virtual bool onPullAtom(const int atomId, std::string* pulledData); virtual void parseArgs(bool asProto, const Vector<String16>& args, std::string& result) = 0; virtual bool isEnabled() = 0; @@ -232,58 +228,11 @@ class TimeStats : public android::TimeStats { public: TimeStats(); - - // Delegate to the statsd service and associated APIs. - // Production code may use this class directly, whereas unit test may define - // a subclass for ease of testing. - class StatsEventDelegate { - public: - virtual ~StatsEventDelegate() = default; - virtual AStatsEvent* addStatsEventToPullData(AStatsEventList* data) { - return AStatsEventList_addStatsEvent(data); - } - virtual void setStatsPullAtomCallback(int32_t atom_tag, - AStatsManager_PullAtomMetadata* metadata, - AStatsManager_PullAtomCallback callback, - void* cookie) { - return AStatsManager_setPullAtomCallback(atom_tag, metadata, callback, cookie); - } - - virtual void clearStatsPullAtomCallback(int32_t atom_tag) { - return AStatsManager_clearPullAtomCallback(atom_tag); - } - - virtual void statsEventSetAtomId(AStatsEvent* event, uint32_t atom_id) { - return AStatsEvent_setAtomId(event, atom_id); - } - - virtual void statsEventWriteInt32(AStatsEvent* event, int32_t field) { - return AStatsEvent_writeInt32(event, field); - } - - virtual void statsEventWriteInt64(AStatsEvent* event, int64_t field) { - return AStatsEvent_writeInt64(event, field); - } - - virtual void statsEventWriteString8(AStatsEvent* event, const char* field) { - return AStatsEvent_writeString(event, field); - } - - virtual void statsEventWriteByteArray(AStatsEvent* event, const uint8_t* buf, - size_t numBytes) { - return AStatsEvent_writeByteArray(event, buf, numBytes); - } - - virtual void statsEventBuild(AStatsEvent* event) { return AStatsEvent_build(event); } - }; // For testing only for injecting custom dependencies. - TimeStats(std::unique_ptr<StatsEventDelegate> statsDelegate, - std::optional<size_t> maxPulledLayers, + TimeStats(std::optional<size_t> maxPulledLayers, std::optional<size_t> maxPulledHistogramBuckets); - ~TimeStats() override; - - void onBootFinished() override; + bool onPullAtom(const int atomId, std::string* pulledData) override; void parseArgs(bool asProto, const Vector<String16>& args, std::string& result) override; bool isEnabled() override; std::string miniDump() override; @@ -332,11 +281,8 @@ public: static const size_t MAX_NUM_TIME_RECORDS = 64; private: - static AStatsManager_PullAtomCallbackReturn pullAtomCallback(int32_t atom_tag, - AStatsEventList* data, - void* cookie); - AStatsManager_PullAtomCallbackReturn populateGlobalAtom(AStatsEventList* data); - AStatsManager_PullAtomCallbackReturn populateLayerAtom(AStatsEventList* data); + bool populateGlobalAtom(std::string* pulledData); + bool populateLayerAtom(std::string* pulledData); bool recordReadyLocked(int32_t layerId, TimeRecord* timeRecord); void flushAvailableRecordsToStatsLocked(int32_t layerId, Fps displayRefreshRate, std::optional<Fps> renderRate, @@ -366,7 +312,6 @@ private: static const size_t RENDER_RATE_BUCKET_WIDTH = REFRESH_RATE_BUCKET_WIDTH; static const size_t MAX_NUM_LAYER_STATS = 200; static const size_t MAX_NUM_PULLED_LAYERS = MAX_NUM_LAYER_STATS; - std::unique_ptr<StatsEventDelegate> mStatsDelegate = std::make_unique<StatsEventDelegate>(); size_t mMaxPulledLayers = MAX_NUM_PULLED_LAYERS; size_t mMaxPulledHistogramBuckets = 6; }; diff --git a/services/surfaceflinger/TimeStats/timestatsatomsproto/Android.bp b/services/surfaceflinger/TimeStats/timestatsatomsproto/Android.bp new file mode 100644 index 0000000000..0cf086fe01 --- /dev/null +++ b/services/surfaceflinger/TimeStats/timestatsatomsproto/Android.bp @@ -0,0 +1,36 @@ +package { + // See: http://go/android-license-faq + // A large-scale-change added 'default_applicable_licenses' to import + // all of the 'license_kinds' from "frameworks_native_license" + // to get the below license kinds: + // SPDX-license-identifier-Apache-2.0 + default_applicable_licenses: ["frameworks_native_license"], +} + +cc_library { + name: "libtimestats_atoms_proto", + export_include_dirs: ["include"], + + srcs: [ + "timestats_atoms.proto", + ], + + proto: { + type: "lite", + export_proto_headers: true, + }, + + cppflags: [ + "-Werror", + "-Wno-c++98-compat-pedantic", + "-Wno-disabled-macro-expansion", + "-Wno-float-conversion", + "-Wno-float-equal", + "-Wno-format", + "-Wno-old-style-cast", + "-Wno-padded", + "-Wno-sign-conversion", + "-Wno-undef", + "-Wno-unused-parameter", + ], +}
\ No newline at end of file diff --git a/services/surfaceflinger/TimeStats/timestatsatomsproto/include/timestatsatomsproto/TimeStatsAtomsProtoHeader.h b/services/surfaceflinger/TimeStats/timestatsatomsproto/include/timestatsatomsproto/TimeStatsAtomsProtoHeader.h new file mode 100644 index 0000000000..d305cb403c --- /dev/null +++ b/services/surfaceflinger/TimeStats/timestatsatomsproto/include/timestatsatomsproto/TimeStatsAtomsProtoHeader.h @@ -0,0 +1,23 @@ +/* + * Copyright 2021 The Android Open Source Project + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +// pragma is used here to disable the warnings emitted from the protobuf +// headers. By adding #pragma before including layer.pb.h, it suppresses +// protobuf warnings, but allows the rest of the files to continuing using +// the current flags. +// This file should be included instead of directly including timestats_atoms.b.h +#pragma GCC system_header +#include <timestats_atoms.pb.h> diff --git a/services/surfaceflinger/TimeStats/timestatsatomsproto/timestats_atoms.proto b/services/surfaceflinger/TimeStats/timestatsatomsproto/timestats_atoms.proto new file mode 100644 index 0000000000..133a5419b5 --- /dev/null +++ b/services/surfaceflinger/TimeStats/timestatsatomsproto/timestats_atoms.proto @@ -0,0 +1,289 @@ +/* + * Copyright 2021 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. + */ +syntax = "proto2"; +option optimize_for = LITE_RUNTIME; +package android.surfaceflinger; + +// This is a copy of surfaceflinger's atoms from frameworks/proto_logging/stats/atoms.proto. +// Pulled atoms for surfaceflinger must be routed through system server since surfaceflinger is +// in the bootstrap namespace. This copy is used to pass the atoms as protos to system server using +// proto lite to serialize/deserialize the atoms. + +// These wrappers are so that we can pass a List<Atom> as a single byte string. +// They are not in atoms.proto +message SurfaceflingerStatsGlobalInfoWrapper { + repeated SurfaceflingerStatsGlobalInfo atom = 1; +} + +message SurfaceflingerStatsLayerInfoWrapper { + repeated SurfaceflingerStatsLayerInfo atom = 1; +} + +/** + * Global display pipeline metrics reported by SurfaceFlinger. + * Metrics exist beginning in Android 11. + * Pulled from: + * frameworks/native/services/surfaceflinger/TimeStats/TimeStats.cpp + */ +message SurfaceflingerStatsGlobalInfo { + // Aggregated refresh rate buckets that layers were presenting at. Buckets + // are defined in SurfaceFlinger and are tracked per device. + // Introduced in Android 12. + // This is intended to be used as a dimenstion in collecting per-refresh rate + // jank statistics. + optional int32 display_refresh_rate_bucket = 18; + // Aggregated render rate buckets that layers were overridden to run at. + // Buckets are defined in SurfaceFlinger and are tracked per device. + // Introduced in Android 12. + // This is intended to be used as a dimension in collecting per-render rate + // jank statistics. + optional int32 render_rate_bucket = 21; + // Total number of frames presented during the tracing period + // Note: This stat is not sliced by dimension. It will be duplicated for metrics + // using render_rate_bucket as a dimension. + optional int64 total_frames = 1; + // Total number of frames missed + // Note: This stat is not sliced by dimension. It will be duplicated for metrics + // using render_rate_bucket as a dimension. + optional int64 missed_frames = 2; + // Total number of frames that fell back to client composition + // Note: This stat is not sliced by dimension. It will be duplicated for metrics + // using render_rate_bucket as a dimension. + optional int64 client_composition_frames = 3; + // Total time the display was turned on + // Note: This stat is not sliced by dimension. It will be duplicated for metrics + // using render_rate_bucket as a dimension. + optional int64 display_on_millis = 4; + // Total time that was spent performing animations. + // This is derived from the present-to-present layer histogram. + // Note: This stat is not sliced by dimension. It will be duplicated for metrics + // using render_rate_bucket as a dimension. + optional int64 animation_millis = 5; + // Total number of event connections tracked by SurfaceFlinger at the time + // of this pull. If this number grows prohibitively large, then this can + // cause jank due to resource contention. + // Note: This stat is not sliced by dimension. It will be duplicated for metrics + // using render_rate_bucket as a dimension. + optional int32 event_connection_count = 6; + // Set of timings measured from when SurfaceFlinger began compositing a + // frame, until the frame was requested to be presented to the display. This + // measures SurfaceFlinger's total CPU walltime on the critical path per + // frame. + // Note: This stat is not sliced by dimension. It will be duplicated for metrics + // using render_rate_bucket as a dimension. + optional FrameTimingHistogram frame_duration = 7; + // Set of timings measured from when SurfaceFlinger first began using the + // GPU to composite a frame, until the GPU has finished compositing that + // frame. This measures the total additional time SurfaceFlinger needed to + // perform due to falling back into GPU composition. + // Note: This stat is not sliced by dimension. It will be duplicated for metrics + // using render_rate_bucket as a dimension. + optional FrameTimingHistogram render_engine_timing = 8; + // Number of frames where SF saw a frame, based on its frame timeline. + // Frame timelines may include transactions without updating buffer contents. + // Introduced in Android 12. + optional int32 total_timeline_frames = 9; + // Number of frames where SF saw a janky frame. + // Introduced in Android 12. + optional int32 total_janky_frames = 10; + // Number of janky frames where SF spent a long time on the CPU. + // Introduced in Android 12. + optional int32 total_janky_frames_with_long_cpu = 11; + // Number of janky frames where SF spent a long time on the GPU. + // Introduced in Android 12. + optional int32 total_janky_frames_with_long_gpu = 12; + // Number of janky frames where SF missed the frame deadline, but there + // was not an attributed reason (e.g., maybe HWC missed?) + // Introduced in Android 12. + optional int32 total_janky_frames_sf_unattributed = 13; + // Number of janky frames where the app missed the frame deadline, but + // there was not an attributed reason + // Introduced in Android 12. + optional int32 total_janky_frames_app_unattributed = 14; + // Number of janky frames that were caused because of scheduling errors in + // SF that resulted in early present (e.g., SF sending a buffer to the + // composition engine earlier than expected, resulting in a present that is + // one vsync early) + // Introduced in Android 12. + optional int32 total_janky_frames_sf_scheduling = 15; + // Number of frames that were classified as jank because of possible drift in + // vsync predictions. + // Introduced in Android 12. + optional int32 total_jank_frames_sf_prediction_error = 16; + // Number of janky frames where the app was in a buffer stuffed state (more + // than one buffer ready to be presented at the same vsync). Usually caused + // when the first frame is unusually long, the following frames enter into a + // stuffed state. + // Introduced in Android 12. + optional int32 total_jank_frames_app_buffer_stuffing = 17; + // Buckets of timings in ms by which SurfaceFlinger's deadline was missed + // while latching and presenting frames. + // Introduced in Android 12. + optional FrameTimingHistogram sf_deadline_misses = 19; + // Buckets of timings in ms by which the Vsync prediction drifted, when + // compared to the actual hardware vsync. + // Introduced in Android 12. + optional FrameTimingHistogram sf_prediction_errors = 20; + + // Next ID: 22 +} + +/** + * Per-layer display pipeline metrics reported by SurfaceFlinger. + * Metrics exist beginning in Android 11. + * The number of layers uploaded may be restricted due to size limitations. + * Pulled from: + * frameworks/native/services/surfaceflinger/TimeStats/TimeStats.cpp + */ +message SurfaceflingerStatsLayerInfo { + // UID of the application who submitted this layer for presentation + // This is intended to be used as a dimension for surfacing rendering + // statistics to applications. + // Introduced in Android 12. + optional int32 uid = 12; + // Refresh rate bucket that the layer was presenting at. Buckets are + // defined in SurfaceFlinger and are tracked per device. + // Introduced in Android 12. + // This is intended to be used as a dimension in collecting per-refresh rate + // jank statistics + optional int32 display_refresh_rate_bucket = 22; + // Render rate bucket that the layer was submitting frames at. Buckets are + // defined in SurfaceFlinger and are tracked per device. + // Introduced in Android 12. + // This is intended to be used as a dimension in collecting per-render rate + // jank statistics. + optional int32 render_rate_bucket = 23; + // The layer for this set of metrics + // In many scenarios the package name is included in the layer name, e.g., + // layers created by Window Manager. But this is not a guarantee - in the + // general case layer names are arbitrary debug names. + optional string layer_name = 1; + // Total number of frames presented + optional int64 total_frames = 2; + // Total number of dropped frames while latching a buffer for this layer. + optional int64 dropped_frames = 3; + // Set of timings measured between successive presentation timestamps. + optional FrameTimingHistogram present_to_present = 4; + // Set of timings measured from when an app queued a buffer for + // presentation, until the buffer was actually presented to the + // display. + optional FrameTimingHistogram post_to_present = 5; + // Set of timings measured from when a buffer is ready to be presented, + // until the buffer was actually presented to the display. + optional FrameTimingHistogram acquire_to_present = 6; + // Set of timings measured from when a buffer was latched by + // SurfaceFlinger, until the buffer was presented to the display + optional FrameTimingHistogram latch_to_present = 7; + // Set of timings measured from the desired presentation to the actual + // presentation time + optional FrameTimingHistogram desired_to_present = 8; + // Set of timings measured from when an app queued a buffer for + // presentation, until the buffer was ready to be presented. + optional FrameTimingHistogram post_to_acquire = 9; + // Frames missed latch because the acquire fence didn't fire + optional int64 late_acquire_frames = 10; + // Frames latched early because the desired present time was bad + optional int64 bad_desired_present_frames = 11; + // Number of frames where SF saw a frame, based on its frame timeline. + // Frame timelines may include transactions without updating buffer contents. + // Introduced in Android 12. + optional int32 total_timeline_frames = 13; + // Number of frames where SF saw a janky frame. + // Introduced in Android 12. + optional int32 total_janky_frames = 14; + // Number of janky frames where SF spent a long time on the CPU. + // Introduced in Android 12. + optional int32 total_janky_frames_with_long_cpu = 15; + // Number of janky frames where SF spent a long time on the GPU. + // Introduced in Android 12. + optional int32 total_janky_frames_with_long_gpu = 16; + // Number of janky frames where SF missed the frame deadline, but there + // was not an attributed reason (e.g., maybe HWC missed?) + // Introduced in Android 12. + optional int32 total_janky_frames_sf_unattributed = 17; + // Number of janky frames where the app missed the frame deadline, but + // there was not an attributed reason + // Introduced in Android 12. + optional int32 total_janky_frames_app_unattributed = 18; + // Number of janky frames that were caused because of scheduling errors in + // SF that resulted in early present (e.g., SF sending a buffer to the + // composition engine earlier than expected, resulting in a present that is + // one vsync early) + // Introduced in Android 12. + optional int32 total_janky_frames_sf_scheduling = 19; + // Number of frames that were classified as jank because of possible drift in + // vsync predictions. + // Introduced in Android 12. + optional int32 total_jank_frames_sf_prediction_error = 20; + // Number of janky frames where the app was in a buffer stuffed state (more + // than one buffer ready to be presented at the same vsync). Usually caused + // when the first frame is unusually long, the following frames enter into a + // stuffed state. + // Introduced in Android 12. + optional int32 total_jank_frames_app_buffer_stuffing = 21; + + /** + * Encapsulates the FrameRateVote information sent by the application while + * calling setFrameRate. + * Metrics exist beginning in Android 12. + */ + message SetFrameRateVote { + // The desired frame rate the application wishes to run on. + optional float frame_rate = 1; + + enum FrameRateCompatibility { + FRAME_RATE_UNDEFINED = 0; + FRAME_RATE_DEFAULT = 1; + FRAME_RATE_EXACT_OR_MULTIPLE = 2; + } + + // Specifies how to interpret the frame rate associated with the layer. + // Defined in Layer.h + optional FrameRateCompatibility frame_rate_compatibility = 2; + + enum Seamlessness { + SEAMLESS_UNDEFINED = 0; + SEAMLESS_SHOULD_BE_SEAMLESS = 1; + SEAMLESS_NOT_REQUIRED = 2; + } + // Indicates whether seamless refresh rate switch is required or not. + optional Seamlessness seamlessness = 3; + } + + // The last frame rate vote set by the application. + // Introduced in Android 12. + optional SetFrameRateVote set_frame_rate_vote = 24; + // Buckets of timings in ms by which the app deadline was missed while + // submitting work for a frame. + // Introduced in Android 12. + optional FrameTimingHistogram app_deadline_misses = 25; + + // Next ID: 26 +} + +/** + * Histogram of frame counts bucketed by time in milliseconds. + * Because of size limitations, we hard-cap the number of buckets, with + * buckets for corresponding to larger milliseconds being less precise. + */ +message FrameTimingHistogram { + // Timings in milliseconds that describes a set of histogram buckets + repeated int32 time_millis_buckets = 1; + // Number of frames that match to each time_millis, i.e. the bucket + // contents + // It's required that len(time_millis) == len(frame_count) + repeated int64 frame_counts = 2; +} diff --git a/services/surfaceflinger/TransactionCallbackInvoker.cpp b/services/surfaceflinger/TransactionCallbackInvoker.cpp index 3590e76cb9..4f4c02be6c 100644 --- a/services/surfaceflinger/TransactionCallbackInvoker.cpp +++ b/services/surfaceflinger/TransactionCallbackInvoker.cpp @@ -36,13 +36,17 @@ namespace android { // <0 if the first id that doesn't match is lower in c2 or all ids match but c2 is shorter // >0 if the first id that doesn't match is greater in c2 or all ids match but c2 is longer // -// See CallbackIdsHash for a explaniation of why this works +// See CallbackIdsHash for a explanation of why this works static int compareCallbackIds(const std::vector<CallbackId>& c1, const std::vector<CallbackId>& c2) { if (c1.empty()) { return !c2.empty(); } - return c1.front() - c2.front(); + return c1.front().id - c2.front().id; +} + +static bool containsOnCommitCallbacks(const std::vector<CallbackId>& callbacks) { + return !callbacks.empty() && callbacks.front().type == CallbackId::Type::ON_COMMIT; } TransactionCallbackInvoker::~TransactionCallbackInvoker() { @@ -114,39 +118,69 @@ status_t TransactionCallbackInvoker::registerPendingCallbackHandle( return NO_ERROR; } -status_t TransactionCallbackInvoker::finalizePendingCallbackHandles( - const std::deque<sp<CallbackHandle>>& handles, const std::vector<JankData>& jankData) { +status_t TransactionCallbackInvoker::finalizeCallbackHandle(const sp<CallbackHandle>& handle, + const std::vector<JankData>& jankData) { + auto listener = mPendingTransactions.find(handle->listener); + if (listener != mPendingTransactions.end()) { + auto& pendingCallbacks = listener->second; + auto pendingCallback = pendingCallbacks.find(handle->callbackIds); + + if (pendingCallback != pendingCallbacks.end()) { + auto& pendingCount = pendingCallback->second; + + // Decrease the pending count for this listener + if (--pendingCount == 0) { + pendingCallbacks.erase(pendingCallback); + } + } else { + ALOGW("there are more latched callbacks than there were registered callbacks"); + } + if (listener->second.size() == 0) { + mPendingTransactions.erase(listener); + } + } else { + ALOGW("cannot find listener in mPendingTransactions"); + } + + status_t err = addCallbackHandle(handle, jankData); + if (err != NO_ERROR) { + ALOGE("could not add callback handle"); + return err; + } + return NO_ERROR; +} + +status_t TransactionCallbackInvoker::finalizeOnCommitCallbackHandles( + const std::deque<sp<CallbackHandle>>& handles, + std::deque<sp<CallbackHandle>>& outRemainingHandles) { if (handles.empty()) { return NO_ERROR; } std::lock_guard lock(mMutex); - + const std::vector<JankData>& jankData = std::vector<JankData>(); for (const auto& handle : handles) { - auto listener = mPendingTransactions.find(handle->listener); - if (listener != mPendingTransactions.end()) { - auto& pendingCallbacks = listener->second; - auto pendingCallback = pendingCallbacks.find(handle->callbackIds); - - if (pendingCallback != pendingCallbacks.end()) { - auto& pendingCount = pendingCallback->second; - - // Decrease the pending count for this listener - if (--pendingCount == 0) { - pendingCallbacks.erase(pendingCallback); - } - } else { - ALOGW("there are more latched callbacks than there were registered callbacks"); - } - if (listener->second.size() == 0) { - mPendingTransactions.erase(listener); - } - } else { - ALOGW("cannot find listener in mPendingTransactions"); + if (!containsOnCommitCallbacks(handle->callbackIds)) { + outRemainingHandles.push_back(handle); + continue; } + status_t err = finalizeCallbackHandle(handle, jankData); + if (err != NO_ERROR) { + return err; + } + } - status_t err = addCallbackHandle(handle, jankData); + return NO_ERROR; +} + +status_t TransactionCallbackInvoker::finalizePendingCallbackHandles( + const std::deque<sp<CallbackHandle>>& handles, const std::vector<JankData>& jankData) { + if (handles.empty()) { + return NO_ERROR; + } + std::lock_guard lock(mMutex); + for (const auto& handle : handles) { + status_t err = finalizeCallbackHandle(handle, jankData); if (err != NO_ERROR) { - ALOGE("could not add callback handle"); return err; } } @@ -243,7 +277,8 @@ void TransactionCallbackInvoker::sendCallbacks() { } // If the transaction has been latched - if (transactionStats.latchTime >= 0) { + if (transactionStats.latchTime >= 0 && + !containsOnCommitCallbacks(transactionStats.callbackIds)) { if (!mPresentFence) { break; } diff --git a/services/surfaceflinger/TransactionCallbackInvoker.h b/services/surfaceflinger/TransactionCallbackInvoker.h index caa8a4fb45..184b15103e 100644 --- a/services/surfaceflinger/TransactionCallbackInvoker.h +++ b/services/surfaceflinger/TransactionCallbackInvoker.h @@ -74,6 +74,8 @@ public: // Notifies the TransactionCallbackInvoker that a pending CallbackHandle has been presented. status_t finalizePendingCallbackHandles(const std::deque<sp<CallbackHandle>>& handles, const std::vector<JankData>& jankData); + status_t finalizeOnCommitCallbackHandles(const std::deque<sp<CallbackHandle>>& handles, + std::deque<sp<CallbackHandle>>& outRemainingHandles); // Adds the Transaction CallbackHandle from a layer that does not need to be relatched and // presented this frame. @@ -95,6 +97,9 @@ private: status_t addCallbackHandle(const sp<CallbackHandle>& handle, const std::vector<JankData>& jankData) REQUIRES(mMutex); + status_t finalizeCallbackHandle(const sp<CallbackHandle>& handle, + const std::vector<JankData>& jankData) REQUIRES(mMutex); + class CallbackDeathRecipient : public IBinder::DeathRecipient { public: // This function is a no-op. isBinderAlive needs a linked DeathRecipient to work. diff --git a/services/surfaceflinger/tests/InvalidHandles_test.cpp b/services/surfaceflinger/tests/InvalidHandles_test.cpp index 58b039e5d5..9cf7c0909b 100644 --- a/services/surfaceflinger/tests/InvalidHandles_test.cpp +++ b/services/surfaceflinger/tests/InvalidHandles_test.cpp @@ -52,12 +52,15 @@ protected: } }; -TEST_F(InvalidHandleTest, createSurfaceInvalidHandle) { - auto notSc = makeNotSurfaceControl(); - ASSERT_EQ(nullptr, - mScc->createSurface(String8("lolcats"), 19, 47, PIXEL_FORMAT_RGBA_8888, 0, - notSc->getHandle()) - .get()); +TEST_F(InvalidHandleTest, createSurfaceInvalidParentHandle) { + // The createSurface is scheduled now, we could still get a created surface from createSurface. + // Should verify if it actually added into current state by checking the screenshot. + auto notSc = mScc->createSurface(String8("lolcats"), 19, 47, PIXEL_FORMAT_RGBA_8888, 0, + mNotSc->getHandle()); + LayerCaptureArgs args; + args.layerHandle = notSc->getHandle(); + ScreenCaptureResults captureResults; + ASSERT_EQ(NAME_NOT_FOUND, ScreenCapture::captureLayers(args, captureResults)); } TEST_F(InvalidHandleTest, captureLayersInvalidHandle) { diff --git a/services/surfaceflinger/tests/unittests/Android.bp b/services/surfaceflinger/tests/unittests/Android.bp index 4ac096b609..88fb811b52 100644 --- a/services/surfaceflinger/tests/unittests/Android.bp +++ b/services/surfaceflinger/tests/unittests/Android.bp @@ -127,6 +127,7 @@ cc_test { "librenderengine", "libserviceutils", "libtimestats", + "libtimestats_atoms_proto", "libtimestats_proto", "libtrace_proto", "perfetto_trace_protos", @@ -152,14 +153,10 @@ cc_test { "libnativewindow", "libprocessgroup", "libprotobuf-cpp-lite", - "libprotoutil", - "libstatslog", - "libstatssocket", "libSurfaceFlingerProp", "libsync", "libui", "libutils", - "libstatspull", ], header_libs: [ "android.hardware.graphics.composer@2.1-command-buffer", diff --git a/services/surfaceflinger/tests/unittests/FrameTimelineTest.cpp b/services/surfaceflinger/tests/unittests/FrameTimelineTest.cpp index 7727052084..8a3f561487 100644 --- a/services/surfaceflinger/tests/unittests/FrameTimelineTest.cpp +++ b/services/surfaceflinger/tests/unittests/FrameTimelineTest.cpp @@ -1347,6 +1347,81 @@ TEST_F(FrameTimelineTest, traceSurfaceFrame_predictionExpiredDoesNotTraceExpecte validateTraceEvent(actualSurfaceFrameEnd, protoActualSurfaceFrameEnd); } +TEST_F(FrameTimelineTest, traceSurfaceFrame_predictionExpiredDroppedFramesTracedProperly) { + auto tracingSession = getTracingSessionForTest(); + auto presentFence1 = fenceFactory.createFenceTimeForTest(Fence::NO_FENCE); + + tracingSession->StartBlocking(); + constexpr nsecs_t appStartTime = std::chrono::nanoseconds(10ms).count(); + constexpr nsecs_t appEndTime = std::chrono::nanoseconds(20ms).count(); + constexpr nsecs_t appPresentTime = std::chrono::nanoseconds(30ms).count(); + int64_t surfaceFrameToken = + mTokenManager->generateTokenForPredictions({appStartTime, appEndTime, appPresentTime}); + + // Flush the token so that it would expire + flushTokens(systemTime() + maxTokenRetentionTime); + auto surfaceFrame1 = + mFrameTimeline->createSurfaceFrameForToken({surfaceFrameToken, /*inputEventId*/ 0}, + sPidOne, sUidOne, sLayerIdOne, sLayerNameOne, + sLayerNameOne, /*isBuffer*/ true); + + constexpr nsecs_t sfStartTime = std::chrono::nanoseconds(22ms).count(); + constexpr nsecs_t sfEndTime = std::chrono::nanoseconds(30ms).count(); + constexpr nsecs_t sfPresentTime = std::chrono::nanoseconds(30ms).count(); + int64_t displayFrameToken = + mTokenManager->generateTokenForPredictions({sfStartTime, sfEndTime, sfPresentTime}); + + // First 2 cookies will be used by the DisplayFrame + int64_t traceCookie = snoopCurrentTraceCookie() + 2; + + auto protoActualSurfaceFrameStart = + createProtoActualSurfaceFrameStart(traceCookie + 1, surfaceFrameToken, + displayFrameToken, sPidOne, sLayerNameOne, + FrameTimelineEvent::PRESENT_DROPPED, false, false, + FrameTimelineEvent::JANK_NONE, + FrameTimelineEvent::PREDICTION_EXPIRED); + auto protoActualSurfaceFrameEnd = createProtoFrameEnd(traceCookie + 1); + + // Set up the display frame + mFrameTimeline->setSfWakeUp(displayFrameToken, sfStartTime, Fps::fromPeriodNsecs(11)); + surfaceFrame1->setDropTime(sfStartTime); + surfaceFrame1->setPresentState(SurfaceFrame::PresentState::Dropped); + mFrameTimeline->addSurfaceFrame(surfaceFrame1); + mFrameTimeline->setSfPresent(sfEndTime, presentFence1); + presentFence1->signalForTest(sfPresentTime); + + addEmptyDisplayFrame(); + flushTrace(); + tracingSession->StopBlocking(); + + auto packets = readFrameTimelinePacketsBlocking(tracingSession.get()); + // Display Frame 4 packets + SurfaceFrame 2 packets + ASSERT_EQ(packets.size(), 6u); + + // Packet - 4 : ActualSurfaceFrameStart + const auto& packet4 = packets[4]; + ASSERT_TRUE(packet4.has_timestamp()); + EXPECT_EQ(packet4.timestamp(), + static_cast<uint64_t>(sfStartTime - SurfaceFrame::kPredictionExpiredStartTimeDelta)); + ASSERT_TRUE(packet4.has_frame_timeline_event()); + + const auto& event4 = packet4.frame_timeline_event(); + ASSERT_TRUE(event4.has_actual_surface_frame_start()); + const auto& actualSurfaceFrameStart = event4.actual_surface_frame_start(); + validateTraceEvent(actualSurfaceFrameStart, protoActualSurfaceFrameStart); + + // Packet - 5 : FrameEnd (ActualSurfaceFrame) + const auto& packet5 = packets[5]; + ASSERT_TRUE(packet5.has_timestamp()); + EXPECT_EQ(packet5.timestamp(), static_cast<uint64_t>(sfStartTime)); + ASSERT_TRUE(packet5.has_frame_timeline_event()); + + const auto& event5 = packet5.frame_timeline_event(); + ASSERT_TRUE(event5.has_frame_end()); + const auto& actualSurfaceFrameEnd = event5.frame_end(); + validateTraceEvent(actualSurfaceFrameEnd, protoActualSurfaceFrameEnd); +} + // Tests for Jank classification TEST_F(FrameTimelineTest, jankClassification_presentOnTimeDoesNotClassify) { // Layer specific increment diff --git a/services/surfaceflinger/tests/unittests/OneShotTimerTest.cpp b/services/surfaceflinger/tests/unittests/OneShotTimerTest.cpp index cfbb3f5e9f..a1f0588fc7 100644 --- a/services/surfaceflinger/tests/unittests/OneShotTimerTest.cpp +++ b/services/surfaceflinger/tests/unittests/OneShotTimerTest.cpp @@ -19,6 +19,7 @@ #include <gmock/gmock.h> #include <gtest/gtest.h> #include <utils/Log.h> +#include <utils/Timers.h> #include "AsyncCallRecorder.h" #include "Scheduler/OneShotTimer.h" @@ -28,21 +29,22 @@ using namespace std::chrono_literals; namespace android { namespace scheduler { +class FakeClock : public OneShotTimer::Clock { +public: + virtual ~FakeClock() = default; + std::chrono::steady_clock::time_point now() const override { return mNow; } + + void advanceTime(std::chrono::nanoseconds delta) { mNow += delta; } + +private: + std::chrono::steady_clock::time_point mNow; +}; + class OneShotTimerTest : public testing::Test { protected: OneShotTimerTest() = default; ~OneShotTimerTest() override = default; - // This timeout should be used when a 3ms callback is expected. - // While the tests typically request a callback after 3ms, the scheduler - // does not always cooperate, at it can take significantly longer (observed - // 30ms). - static constexpr auto waitTimeForExpected3msCallback = 100ms; - - // This timeout should be used when an 3ms callback is not expected. - // Note that there can be false-negatives if the callback happens later. - static constexpr auto waitTimeForUnexpected3msCallback = 6ms; - AsyncCallRecorder<void (*)()> mResetTimerCallback; AsyncCallRecorder<void (*)()> mExpiredTimerCallback; @@ -56,162 +58,179 @@ protected: namespace { TEST_F(OneShotTimerTest, createAndDestroyTest) { + FakeClock* clock = new FakeClock(); mIdleTimer = std::make_unique<scheduler::OneShotTimer>( - "TestTimer", 3ms, [] {}, [] {}); + "TestTimer", 3ms, [] {}, [] {}, std::unique_ptr<FakeClock>(clock)); } TEST_F(OneShotTimerTest, startStopTest) { - mIdleTimer = std::make_unique<scheduler::OneShotTimer>("TestTimer", 30ms, + FakeClock* clock = new FakeClock(); + mIdleTimer = std::make_unique<scheduler::OneShotTimer>("TestTimer", 1ms, mResetTimerCallback.getInvocable(), - mExpiredTimerCallback.getInvocable()); - auto startTime = std::chrono::steady_clock::now(); + mExpiredTimerCallback.getInvocable(), + std::unique_ptr<FakeClock>(clock)); mIdleTimer->start(); EXPECT_TRUE(mResetTimerCallback.waitForCall().has_value()); - // The idle timer fires after 30ms, so there should be no callback within - // 25ms (waiting for a callback for the full 30ms would be problematic). - bool callbackCalled = mExpiredTimerCallback.waitForCall(25ms).has_value(); - // Under ideal conditions there should be no event. But occasionally - // it is possible that the wait just prior takes more than 30ms, and - // a callback is observed. We check the elapsed time since before the OneShotTimer - // thread was started as a sanity check to not have a flakey test. - EXPECT_FALSE(callbackCalled && std::chrono::steady_clock::now() - startTime < 30ms); - - std::this_thread::sleep_for(std::chrono::milliseconds(25)); - EXPECT_FALSE(mResetTimerCallback.waitForCall().has_value()); + EXPECT_FALSE(mExpiredTimerCallback.waitForUnexpectedCall().has_value()); + + clock->advanceTime(2ms); + EXPECT_TRUE(mExpiredTimerCallback.waitForCall().has_value()); + + clock->advanceTime(2ms); + EXPECT_FALSE(mExpiredTimerCallback.waitForUnexpectedCall().has_value()); + EXPECT_FALSE(mResetTimerCallback.waitForUnexpectedCall().has_value()); mIdleTimer->stop(); } TEST_F(OneShotTimerTest, resetTest) { - mIdleTimer = std::make_unique<scheduler::OneShotTimer>("TestTimer", 20ms, + FakeClock* clock = new FakeClock(); + mIdleTimer = std::make_unique<scheduler::OneShotTimer>("TestTimer", 1ms, mResetTimerCallback.getInvocable(), - mExpiredTimerCallback.getInvocable()); + mExpiredTimerCallback.getInvocable(), + std::unique_ptr<FakeClock>(clock)); + mIdleTimer->start(); EXPECT_TRUE(mResetTimerCallback.waitForCall().has_value()); - // Observe any event that happens in about 25ms. We don't care if one was - // observed or not. - mExpiredTimerCallback.waitForCall(25ms).has_value(); + EXPECT_FALSE(mExpiredTimerCallback.waitForUnexpectedCall().has_value()); + clock->advanceTime(2ms); + EXPECT_TRUE(mExpiredTimerCallback.waitForCall().has_value()); mIdleTimer->reset(); EXPECT_TRUE(mResetTimerCallback.waitForCall().has_value()); - // There may have been a race with the reset. Clear any callbacks we - // received right afterwards. - clearPendingCallbacks(); - // A single callback should be generated after 30ms - EXPECT_TRUE( - mExpiredTimerCallback.waitForCall(waitTimeForExpected3msCallback + 30ms).has_value()); - // After one event, it should be idle, and not generate another. - EXPECT_FALSE( - mExpiredTimerCallback.waitForCall(waitTimeForUnexpected3msCallback * 10).has_value()); - mIdleTimer->stop(); - // Final quick check that no more callback were observed. - EXPECT_FALSE(mExpiredTimerCallback.waitForCall(0ms).has_value()); - EXPECT_FALSE(mResetTimerCallback.waitForCall(0ms).has_value()); + clock->advanceTime(2ms); + EXPECT_TRUE(mExpiredTimerCallback.waitForCall().has_value()); + + clock->advanceTime(2ms); + EXPECT_FALSE(mExpiredTimerCallback.waitForUnexpectedCall().has_value()); + EXPECT_FALSE(mResetTimerCallback.waitForUnexpectedCall().has_value()); } TEST_F(OneShotTimerTest, resetBackToBackTest) { - mIdleTimer = std::make_unique<scheduler::OneShotTimer>("TestTimer", 20ms, + FakeClock* clock = new FakeClock(); + mIdleTimer = std::make_unique<scheduler::OneShotTimer>("TestTimer", 1ms, mResetTimerCallback.getInvocable(), - mExpiredTimerCallback.getInvocable()); + mExpiredTimerCallback.getInvocable(), + std::unique_ptr<FakeClock>(clock)); mIdleTimer->start(); EXPECT_TRUE(mResetTimerCallback.waitForCall().has_value()); mIdleTimer->reset(); - EXPECT_FALSE(mResetTimerCallback.waitForCall(1ms).has_value()); - EXPECT_FALSE(mExpiredTimerCallback.waitForCall(waitTimeForUnexpected3msCallback).has_value()); + EXPECT_FALSE(mResetTimerCallback.waitForUnexpectedCall().has_value()); + EXPECT_FALSE(mExpiredTimerCallback.waitForUnexpectedCall().has_value()); mIdleTimer->reset(); - EXPECT_FALSE(mResetTimerCallback.waitForCall(1ms).has_value()); - EXPECT_FALSE(mExpiredTimerCallback.waitForCall(waitTimeForUnexpected3msCallback).has_value()); + EXPECT_FALSE(mResetTimerCallback.waitForUnexpectedCall().has_value()); + EXPECT_FALSE(mExpiredTimerCallback.waitForUnexpectedCall().has_value()); mIdleTimer->reset(); - EXPECT_FALSE(mResetTimerCallback.waitForCall(1ms).has_value()); - EXPECT_FALSE(mExpiredTimerCallback.waitForCall(waitTimeForUnexpected3msCallback).has_value()); + EXPECT_FALSE(mResetTimerCallback.waitForUnexpectedCall().has_value()); + EXPECT_FALSE(mExpiredTimerCallback.waitForUnexpectedCall().has_value()); mIdleTimer->reset(); - EXPECT_FALSE(mResetTimerCallback.waitForCall(1ms).has_value()); - EXPECT_FALSE(mExpiredTimerCallback.waitForCall(waitTimeForUnexpected3msCallback).has_value()); + EXPECT_FALSE(mResetTimerCallback.waitForUnexpectedCall().has_value()); + + clock->advanceTime(2ms); + EXPECT_TRUE(mExpiredTimerCallback.waitForCall().has_value()); - // A single callback should be generated after 30ms - EXPECT_TRUE( - mExpiredTimerCallback.waitForCall(waitTimeForExpected3msCallback + 30ms).has_value()); mIdleTimer->stop(); + clock->advanceTime(2ms); // Final quick check that no more callback were observed. - EXPECT_FALSE(mExpiredTimerCallback.waitForCall(0ms).has_value()); - EXPECT_FALSE(mResetTimerCallback.waitForCall(0ms).has_value()); + EXPECT_FALSE(mExpiredTimerCallback.waitForUnexpectedCall().has_value()); + EXPECT_FALSE(mResetTimerCallback.waitForUnexpectedCall().has_value()); } TEST_F(OneShotTimerTest, startNotCalledTest) { - mIdleTimer = std::make_unique<scheduler::OneShotTimer>("TestTimer", 3ms, + FakeClock* clock = new FakeClock(); + mIdleTimer = std::make_unique<scheduler::OneShotTimer>("TestTimer", 1ms, mResetTimerCallback.getInvocable(), - mExpiredTimerCallback.getInvocable()); + mExpiredTimerCallback.getInvocable(), + std::unique_ptr<FakeClock>(clock)); // The start hasn't happened, so the callback does not happen. - EXPECT_FALSE(mExpiredTimerCallback.waitForCall(waitTimeForUnexpected3msCallback).has_value()); - EXPECT_FALSE(mResetTimerCallback.waitForCall().has_value()); + EXPECT_FALSE(mExpiredTimerCallback.waitForUnexpectedCall().has_value()); + EXPECT_FALSE(mResetTimerCallback.waitForUnexpectedCall().has_value()); mIdleTimer->stop(); + clock->advanceTime(2ms); // Final quick check that no more callback were observed. - EXPECT_FALSE(mExpiredTimerCallback.waitForCall(0ms).has_value()); - EXPECT_FALSE(mResetTimerCallback.waitForCall(0ms).has_value()); + EXPECT_FALSE(mExpiredTimerCallback.waitForUnexpectedCall().has_value()); + EXPECT_FALSE(mResetTimerCallback.waitForUnexpectedCall().has_value()); } TEST_F(OneShotTimerTest, idleTimerIdlesTest) { - mIdleTimer = std::make_unique<scheduler::OneShotTimer>("TestTimer", 3ms, + FakeClock* clock = new FakeClock(); + mIdleTimer = std::make_unique<scheduler::OneShotTimer>("TestTimer", 1ms, mResetTimerCallback.getInvocable(), - mExpiredTimerCallback.getInvocable()); + mExpiredTimerCallback.getInvocable(), + std::unique_ptr<FakeClock>(clock)); mIdleTimer->start(); EXPECT_TRUE(mResetTimerCallback.waitForCall().has_value()); + clock->advanceTime(2ms); + EXPECT_TRUE(mExpiredTimerCallback.waitForCall().has_value()); + + EXPECT_FALSE(mExpiredTimerCallback.waitForUnexpectedCall().has_value()); + EXPECT_FALSE(mResetTimerCallback.waitForUnexpectedCall().has_value()); - // A callback should be generated after 3ms - EXPECT_TRUE(mExpiredTimerCallback.waitForCall(waitTimeForExpected3msCallback).has_value()); - // After one event, it should be idle, and not generate another. - EXPECT_FALSE(mExpiredTimerCallback.waitForCall(waitTimeForUnexpected3msCallback).has_value()); - // Once reset, it should generate another mIdleTimer->reset(); EXPECT_TRUE(mResetTimerCallback.waitForCall().has_value()); - EXPECT_TRUE(mExpiredTimerCallback.waitForCall(waitTimeForExpected3msCallback).has_value()); + clock->advanceTime(2ms); + EXPECT_TRUE(mExpiredTimerCallback.waitForCall().has_value()); mIdleTimer->stop(); + clock->advanceTime(2ms); // Final quick check that no more callback were observed. - EXPECT_FALSE(mExpiredTimerCallback.waitForCall(0ms).has_value()); - EXPECT_FALSE(mResetTimerCallback.waitForCall(0ms).has_value()); + EXPECT_FALSE(mExpiredTimerCallback.waitForUnexpectedCall().has_value()); + EXPECT_FALSE(mResetTimerCallback.waitForUnexpectedCall().has_value()); } TEST_F(OneShotTimerTest, timeoutCallbackExecutionTest) { - mIdleTimer = std::make_unique<scheduler::OneShotTimer>("TestTimer", 3ms, + FakeClock* clock = new FakeClock(); + mIdleTimer = std::make_unique<scheduler::OneShotTimer>("TestTimer", 1ms, mResetTimerCallback.getInvocable(), - mExpiredTimerCallback.getInvocable()); + mExpiredTimerCallback.getInvocable(), + std::unique_ptr<FakeClock>(clock)); mIdleTimer->start(); EXPECT_TRUE(mResetTimerCallback.waitForCall().has_value()); - EXPECT_TRUE(mExpiredTimerCallback.waitForCall(waitTimeForExpected3msCallback).has_value()); + + clock->advanceTime(2ms); + EXPECT_TRUE(mExpiredTimerCallback.waitForCall().has_value()); mIdleTimer->stop(); + clock->advanceTime(2ms); + EXPECT_FALSE(mExpiredTimerCallback.waitForUnexpectedCall().has_value()); + EXPECT_FALSE(mResetTimerCallback.waitForUnexpectedCall().has_value()); } TEST_F(OneShotTimerTest, noCallbacksAfterStopAndResetTest) { - mIdleTimer = std::make_unique<scheduler::OneShotTimer>("TestTimer", 3ms, + FakeClock* clock = new FakeClock(); + mIdleTimer = std::make_unique<scheduler::OneShotTimer>("TestTimer", 1ms, mResetTimerCallback.getInvocable(), - mExpiredTimerCallback.getInvocable()); + mExpiredTimerCallback.getInvocable(), + std::unique_ptr<FakeClock>(clock)); mIdleTimer->start(); EXPECT_TRUE(mResetTimerCallback.waitForCall().has_value()); - EXPECT_TRUE(mExpiredTimerCallback.waitForCall(waitTimeForExpected3msCallback).has_value()); + clock->advanceTime(2ms); + EXPECT_TRUE(mExpiredTimerCallback.waitForCall().has_value()); mIdleTimer->stop(); - clearPendingCallbacks(); mIdleTimer->reset(); - EXPECT_FALSE(mExpiredTimerCallback.waitForCall(waitTimeForUnexpected3msCallback).has_value()); - EXPECT_FALSE(mResetTimerCallback.waitForCall().has_value()); + clock->advanceTime(2ms); + EXPECT_FALSE(mExpiredTimerCallback.waitForUnexpectedCall().has_value()); + EXPECT_FALSE(mResetTimerCallback.waitForUnexpectedCall().has_value()); } TEST_F(OneShotTimerTest, noCallbacksAfterStopTest) { - mIdleTimer = std::make_unique<scheduler::OneShotTimer>("TestTimer", 3ms, + FakeClock* clock = new FakeClock(); + mIdleTimer = std::make_unique<scheduler::OneShotTimer>("TestTimer", 1ms, mResetTimerCallback.getInvocable(), - mExpiredTimerCallback.getInvocable()); + mExpiredTimerCallback.getInvocable(), + std::unique_ptr<FakeClock>(clock)); mIdleTimer->start(); EXPECT_TRUE(mResetTimerCallback.waitForCall().has_value()); + EXPECT_FALSE(mExpiredTimerCallback.waitForUnexpectedCall().has_value()); mIdleTimer->stop(); - clearPendingCallbacks(); mIdleTimer->reset(); + clock->advanceTime(2ms); // No more idle events should be observed - EXPECT_FALSE(mExpiredTimerCallback.waitForCall(waitTimeForUnexpected3msCallback).has_value()); - EXPECT_FALSE(mResetTimerCallback.waitForCall().has_value()); + EXPECT_FALSE(mExpiredTimerCallback.waitForUnexpectedCall().has_value()); + EXPECT_FALSE(mResetTimerCallback.waitForUnexpectedCall().has_value()); } } // namespace diff --git a/services/surfaceflinger/tests/unittests/TimeStatsTest.cpp b/services/surfaceflinger/tests/unittests/TimeStatsTest.cpp index c1f0c4ef03..188ea758d4 100644 --- a/services/surfaceflinger/tests/unittests/TimeStatsTest.cpp +++ b/services/surfaceflinger/tests/unittests/TimeStatsTest.cpp @@ -23,10 +23,10 @@ #define LOG_TAG "LibSurfaceFlingerUnittests" #include <TimeStats/TimeStats.h> -#include <android/util/ProtoOutputStream.h> #include <gmock/gmock.h> #include <gtest/gtest.h> #include <log/log.h> +#include <timestatsatomsproto/TimeStatsAtomsProtoHeader.h> #include <utils/String16.h> #include <utils/Vector.h> @@ -156,44 +156,8 @@ public: } std::mt19937 mRandomEngine = std::mt19937(std::random_device()()); - - class FakeStatsEventDelegate : public impl::TimeStats::StatsEventDelegate { - public: - FakeStatsEventDelegate() = default; - ~FakeStatsEventDelegate() override = default; - - struct AStatsEvent* addStatsEventToPullData(AStatsEventList*) override { - return mEvent; - } - void setStatsPullAtomCallback(int32_t atom_tag, AStatsManager_PullAtomMetadata*, - AStatsManager_PullAtomCallback callback, - void* cookie) override { - mAtomTags.push_back(atom_tag); - mCallback = callback; - mCookie = cookie; - } - - AStatsManager_PullAtomCallbackReturn makePullAtomCallback(int32_t atom_tag, void* cookie) { - return (*mCallback)(atom_tag, nullptr, cookie); - } - - MOCK_METHOD1(clearStatsPullAtomCallback, void(int32_t)); - MOCK_METHOD2(statsEventSetAtomId, void(AStatsEvent*, uint32_t)); - MOCK_METHOD2(statsEventWriteInt32, void(AStatsEvent*, int32_t)); - MOCK_METHOD2(statsEventWriteInt64, void(AStatsEvent*, int64_t)); - MOCK_METHOD2(statsEventWriteString8, void(AStatsEvent*, const char*)); - MOCK_METHOD3(statsEventWriteByteArray, void(AStatsEvent*, const uint8_t*, size_t)); - MOCK_METHOD1(statsEventBuild, void(AStatsEvent*)); - - AStatsEvent* mEvent = AStatsEvent_obtain(); - std::vector<int32_t> mAtomTags; - AStatsManager_PullAtomCallback mCallback = nullptr; - void* mCookie = nullptr; - }; - FakeStatsEventDelegate* mDelegate = new FakeStatsEventDelegate; std::unique_ptr<TimeStats> mTimeStats = - std::make_unique<impl::TimeStats>(std::unique_ptr<FakeStatsEventDelegate>(mDelegate), - std::nullopt, std::nullopt); + std::make_unique<impl::TimeStats>(std::nullopt, std::nullopt); }; std::string TimeStatsTest::inputCommand(InputCommand cmd, bool useProto) { @@ -278,21 +242,6 @@ TEST_F(TimeStatsTest, disabledByDefault) { ASSERT_FALSE(mTimeStats->isEnabled()); } -TEST_F(TimeStatsTest, setsCallbacksAfterBoot) { - mTimeStats->onBootFinished(); - EXPECT_THAT(mDelegate->mAtomTags, - UnorderedElementsAre(android::util::SURFACEFLINGER_STATS_GLOBAL_INFO, - android::util::SURFACEFLINGER_STATS_LAYER_INFO)); -} - -TEST_F(TimeStatsTest, clearsCallbacksOnDestruction) { - EXPECT_CALL(*mDelegate, - clearStatsPullAtomCallback(android::util::SURFACEFLINGER_STATS_GLOBAL_INFO)); - EXPECT_CALL(*mDelegate, - clearStatsPullAtomCallback(android::util::SURFACEFLINGER_STATS_LAYER_INFO)); - mTimeStats.reset(); -} - TEST_F(TimeStatsTest, canEnableAndDisableTimeStats) { EXPECT_TRUE(inputCommand(InputCommand::ENABLE, FMT_STRING).empty()); ASSERT_TRUE(mTimeStats->isEnabled()); @@ -1012,61 +961,49 @@ TEST_F(TimeStatsTest, noInfInAverageFPS) { } namespace { -std::string buildExpectedHistogramBytestring(const std::vector<int32_t>& times, - const std::vector<int32_t>& frameCounts) { - util::ProtoOutputStream proto; +FrameTimingHistogram buildExpectedHistogram(const std::vector<int32_t>& times, + const std::vector<int32_t>& frameCounts) { + FrameTimingHistogram histogram; for (int i = 0; i < times.size(); i++) { ALOGE("Writing time: %d", times[i]); - proto.write(util::FIELD_TYPE_INT32 | util::FIELD_COUNT_REPEATED | 1 /* field id */, - (int32_t)times[i]); + histogram.add_time_millis_buckets(times[i]); ALOGE("Writing count: %d", frameCounts[i]); - proto.write(util::FIELD_TYPE_INT64 | util::FIELD_COUNT_REPEATED | 2 /* field id */, - (int64_t)frameCounts[i]); - } - std::string byteString; - proto.serializeToString(&byteString); - return byteString; -} - -std::string frameRateVoteToProtoByteString( - float refreshRate, - TimeStats::SetFrameRateVote::FrameRateCompatibility frameRateCompatibility, - TimeStats::SetFrameRateVote::Seamlessness seamlessness) { - util::ProtoOutputStream proto; - proto.write(android::util::FIELD_TYPE_FLOAT | 1 /* field id */, refreshRate); - proto.write(android::util::FIELD_TYPE_ENUM | 2 /* field id */, - static_cast<int>(frameRateCompatibility)); - proto.write(android::util::FIELD_TYPE_ENUM | 3 /* field id */, static_cast<int>(seamlessness)); - - std::string byteString; - proto.serializeToString(&byteString); - return byteString; -} - -std::string dumpByteStringHex(const std::string& str) { - std::stringstream ss; - ss << std::hex; - for (const char& c : str) { - ss << (int)c << " "; + histogram.add_frame_counts((int64_t)frameCounts[i]); } - - return ss.str(); + return histogram; } - } // namespace -MATCHER_P2(BytesEq, bytes, size, "") { - std::string expected; - expected.append((const char*)bytes, size); - std::string actual; - actual.append((const char*)arg, size); +MATCHER_P(HistogramEq, expected, "") { + *result_listener << "Histograms are not equal! \n"; - *result_listener << "Bytes are not equal! \n"; - *result_listener << "size: " << size << "\n"; - *result_listener << "expected: " << dumpByteStringHex(expected).c_str() << "\n"; - *result_listener << "actual: " << dumpByteStringHex(actual).c_str() << "\n"; + if (arg.time_millis_buckets_size() != expected.time_millis_buckets_size()) { + *result_listener << "Time millis bucket are different sizes. Expected: " + << expected.time_millis_buckets_size() << ". Actual " + << arg.time_millis_buckets_size(); + return false; + } + if (arg.frame_counts_size() != expected.frame_counts_size()) { + *result_listener << "Frame counts are different sizes. Expected: " + << expected.frame_counts_size() << ". Actual " << arg.frame_counts_size(); + return false; + } - return expected == actual; + for (int i = 0; i < expected.time_millis_buckets_size(); i++) { + if (arg.time_millis_buckets(i) != expected.time_millis_buckets(i)) { + *result_listener << "time_millis_bucket[" << i + << "] is different. Expected: " << expected.time_millis_buckets(i) + << ". Actual: " << arg.time_millis_buckets(i); + return false; + } + if (arg.frame_counts(i) != expected.frame_counts(i)) { + *result_listener << "frame_counts[" << i + << "] is different. Expected: " << expected.frame_counts(i) + << ". Actual: " << arg.frame_counts(i); + return false; + } + } + return true; } TEST_F(TimeStatsTest, globalStatsCallback) { @@ -1075,7 +1012,6 @@ TEST_F(TimeStatsTest, globalStatsCallback) { constexpr size_t CLIENT_COMPOSITION_FRAMES = 3; constexpr size_t DISPLAY_EVENT_CONNECTIONS = 14; - mTimeStats->onBootFinished(); EXPECT_TRUE(inputCommand(InputCommand::ENABLE, FMT_STRING).empty()); for (size_t i = 0; i < TOTAL_FRAMES; i++) { @@ -1115,71 +1051,39 @@ TEST_F(TimeStatsTest, globalStatsCallback) { JankType::AppDeadlineMissed | JankType::BufferStuffing, 1, 2, 3}); mTimeStats->incrementJankyFrames({kRefreshRate0, kRenderRate0, UID_0, genLayerName(LAYER_ID_0), + JankType::BufferStuffing, 1, 2, 3}); + mTimeStats->incrementJankyFrames({kRefreshRate0, kRenderRate0, UID_0, genLayerName(LAYER_ID_0), JankType::None, 1, 2, 3}); - EXPECT_THAT(mDelegate->mAtomTags, - UnorderedElementsAre(android::util::SURFACEFLINGER_STATS_GLOBAL_INFO, - android::util::SURFACEFLINGER_STATS_LAYER_INFO)); - EXPECT_NE(nullptr, mDelegate->mCallback); - EXPECT_EQ(mTimeStats.get(), mDelegate->mCookie); - - std::string expectedFrameDuration = buildExpectedHistogramBytestring({2}, {1}); - std::string expectedRenderEngineTiming = buildExpectedHistogramBytestring({1, 2}, {1, 1}); - std::string expectedEmptyHistogram = buildExpectedHistogramBytestring({}, {}); - std::string expectedSfDeadlineMissed = buildExpectedHistogramBytestring({1}, {7}); - std::string expectedSfPredictionErrors = buildExpectedHistogramBytestring({2}, {7}); - - { - InSequence seq; - EXPECT_CALL(*mDelegate, - statsEventSetAtomId(mDelegate->mEvent, - android::util::SURFACEFLINGER_STATS_GLOBAL_INFO)); - EXPECT_CALL(*mDelegate, statsEventWriteInt64(mDelegate->mEvent, TOTAL_FRAMES)); - EXPECT_CALL(*mDelegate, statsEventWriteInt64(mDelegate->mEvent, MISSED_FRAMES)); - EXPECT_CALL(*mDelegate, statsEventWriteInt64(mDelegate->mEvent, CLIENT_COMPOSITION_FRAMES)); - EXPECT_CALL(*mDelegate, statsEventWriteInt64(mDelegate->mEvent, _)); - EXPECT_CALL(*mDelegate, statsEventWriteInt64(mDelegate->mEvent, 2)); - EXPECT_CALL(*mDelegate, statsEventWriteInt32(mDelegate->mEvent, DISPLAY_EVENT_CONNECTIONS)); - EXPECT_CALL(*mDelegate, - statsEventWriteByteArray(mDelegate->mEvent, - BytesEq((const uint8_t*)expectedFrameDuration.c_str(), - expectedFrameDuration.size()), - expectedFrameDuration.size())); - EXPECT_CALL(*mDelegate, - statsEventWriteByteArray(mDelegate->mEvent, - BytesEq((const uint8_t*) - expectedRenderEngineTiming.c_str(), - expectedRenderEngineTiming.size()), - expectedRenderEngineTiming.size())); - EXPECT_CALL(*mDelegate, statsEventWriteInt32(mDelegate->mEvent, 8)); - EXPECT_CALL(*mDelegate, statsEventWriteInt32(mDelegate->mEvent, 7)); - EXPECT_CALL(*mDelegate, statsEventWriteInt32(mDelegate->mEvent, 1)); - EXPECT_CALL(*mDelegate, statsEventWriteInt32(mDelegate->mEvent, 1)); - EXPECT_CALL(*mDelegate, statsEventWriteInt32(mDelegate->mEvent, 1)); - EXPECT_CALL(*mDelegate, statsEventWriteInt32(mDelegate->mEvent, 2)); - EXPECT_CALL(*mDelegate, statsEventWriteInt32(mDelegate->mEvent, 1)); - EXPECT_CALL(*mDelegate, statsEventWriteInt32(mDelegate->mEvent, 1)); - EXPECT_CALL(*mDelegate, statsEventWriteInt32(mDelegate->mEvent, 1)); - EXPECT_CALL(*mDelegate, statsEventWriteInt32(mDelegate->mEvent, REFRESH_RATE_BUCKET_0)); - EXPECT_CALL(*mDelegate, - statsEventWriteByteArray(mDelegate->mEvent, - BytesEq((const uint8_t*) - expectedSfDeadlineMissed.c_str(), - expectedSfDeadlineMissed.size()), - expectedSfDeadlineMissed.size())); - EXPECT_CALL(*mDelegate, - statsEventWriteByteArray(mDelegate->mEvent, - BytesEq((const uint8_t*) - expectedSfPredictionErrors.c_str(), - expectedSfPredictionErrors.size()), - expectedSfPredictionErrors.size())); - EXPECT_CALL(*mDelegate, statsEventWriteInt32(mDelegate->mEvent, RENDER_RATE_BUCKET_0)); - - EXPECT_CALL(*mDelegate, statsEventBuild(mDelegate->mEvent)); - } - EXPECT_EQ(AStatsManager_PULL_SUCCESS, - mDelegate->makePullAtomCallback(android::util::SURFACEFLINGER_STATS_GLOBAL_INFO, - mDelegate->mCookie)); + std::string pulledData; + EXPECT_TRUE(mTimeStats->onPullAtom(10062 /*SURFACEFLINGER_STATS_GLOBAL_INFO*/, &pulledData)); + + android::surfaceflinger::SurfaceflingerStatsGlobalInfoWrapper atomList; + ASSERT_TRUE(atomList.ParseFromString(pulledData)); + ASSERT_EQ(atomList.atom_size(), 1); + const android::surfaceflinger::SurfaceflingerStatsGlobalInfo& atom = atomList.atom(0); + + EXPECT_EQ(atom.total_frames(), TOTAL_FRAMES); + EXPECT_EQ(atom.missed_frames(), MISSED_FRAMES); + EXPECT_EQ(atom.client_composition_frames(), CLIENT_COMPOSITION_FRAMES); + // Display on millis is not checked. + EXPECT_EQ(atom.animation_millis(), 2); + EXPECT_EQ(atom.event_connection_count(), DISPLAY_EVENT_CONNECTIONS); + EXPECT_THAT(atom.frame_duration(), HistogramEq(buildExpectedHistogram({2}, {1}))); + EXPECT_THAT(atom.render_engine_timing(), HistogramEq(buildExpectedHistogram({1, 2}, {1, 1}))); + EXPECT_EQ(atom.total_timeline_frames(), 9); + EXPECT_EQ(atom.total_janky_frames(), 7); + EXPECT_EQ(atom.total_janky_frames_with_long_cpu(), 1); + EXPECT_EQ(atom.total_janky_frames_with_long_gpu(), 1); + EXPECT_EQ(atom.total_janky_frames_sf_unattributed(), 1); + EXPECT_EQ(atom.total_janky_frames_app_unattributed(), 2); + EXPECT_EQ(atom.total_janky_frames_sf_scheduling(), 1); + EXPECT_EQ(atom.total_jank_frames_sf_prediction_error(), 1); + EXPECT_EQ(atom.total_jank_frames_app_buffer_stuffing(), 2); + EXPECT_EQ(atom.display_refresh_rate_bucket(), REFRESH_RATE_BUCKET_0); + EXPECT_THAT(atom.sf_deadline_misses(), HistogramEq(buildExpectedHistogram({1}, {7}))); + EXPECT_THAT(atom.sf_prediction_errors(), HistogramEq(buildExpectedHistogram({2}, {7}))); + EXPECT_EQ(atom.render_rate_bucket(), RENDER_RATE_BUCKET_0); SFTimeStatsGlobalProto globalProto; ASSERT_TRUE(globalProto.ParseFromString(inputCommand(InputCommand::DUMP_ALL, FMT_PROTO))); @@ -1194,7 +1098,7 @@ TEST_F(TimeStatsTest, globalStatsCallback) { const std::string result(inputCommand(InputCommand::DUMP_ALL, FMT_STRING)); std::string expectedResult = "totalTimelineFrames = " + std::to_string(0); EXPECT_THAT(result, HasSubstr(expectedResult)); - expectedResult = "totalTimelineFrames = " + std::to_string(8); + expectedResult = "totalTimelineFrames = " + std::to_string(9); EXPECT_THAT(result, HasSubstr(expectedResult)); expectedResult = "jankyFrames = " + std::to_string(0); EXPECT_THAT(result, HasSubstr(expectedResult)); @@ -1226,7 +1130,7 @@ TEST_F(TimeStatsTest, globalStatsCallback) { EXPECT_THAT(result, HasSubstr(expectedResult)); expectedResult = "appBufferStuffingJankyFrames = " + std::to_string(0); EXPECT_THAT(result, HasSubstr(expectedResult)); - expectedResult = "appBufferStuffingJankyFrames = " + std::to_string(1); + expectedResult = "appBufferStuffingJankyFrames = " + std::to_string(2); EXPECT_THAT(result, HasSubstr(expectedResult)); } @@ -1235,8 +1139,6 @@ TEST_F(TimeStatsTest, layerStatsCallback_pullsAllAndClears) { constexpr size_t BAD_DESIRED_PRESENT_FRAMES = 3; EXPECT_TRUE(inputCommand(InputCommand::ENABLE, FMT_STRING).empty()); - mTimeStats->onBootFinished(); - insertTimeRecord(NORMAL_SEQUENCE, LAYER_ID_0, 1, 1000000); for (size_t i = 0; i < LATE_ACQUIRE_FRAMES; i++) { mTimeStats->incrementLatchSkipped(LAYER_ID_0, TimeStats::LatchSkipReason::LateAcquire); @@ -1270,99 +1172,42 @@ TEST_F(TimeStatsTest, layerStatsCallback_pullsAllAndClears) { mTimeStats->incrementJankyFrames({kRefreshRate0, kRenderRate0, UID_0, genLayerName(LAYER_ID_0), JankType::None, 1, 2, 3}); - EXPECT_THAT(mDelegate->mAtomTags, - UnorderedElementsAre(android::util::SURFACEFLINGER_STATS_GLOBAL_INFO, - android::util::SURFACEFLINGER_STATS_LAYER_INFO)); - EXPECT_NE(nullptr, mDelegate->mCallback); - EXPECT_EQ(mTimeStats.get(), mDelegate->mCookie); - - std::string expectedPresentToPresent = buildExpectedHistogramBytestring({1}, {1}); - std::string expectedPostToPresent = buildExpectedHistogramBytestring({4}, {1}); - std::string expectedAcquireToPresent = buildExpectedHistogramBytestring({3}, {1}); - std::string expectedLatchToPresent = buildExpectedHistogramBytestring({2}, {1}); - std::string expectedDesiredToPresent = buildExpectedHistogramBytestring({1}, {1}); - std::string expectedPostToAcquire = buildExpectedHistogramBytestring({1}, {1}); - std::string expectedFrameRateOverride = - frameRateVoteToProtoByteString(frameRate60.frameRate, - frameRate60.frameRateCompatibility, - frameRate60.seamlessness); - std::string expectedAppDeadlineMissed = buildExpectedHistogramBytestring({3, 2}, {4, 3}); - { - InSequence seq; - EXPECT_CALL(*mDelegate, - statsEventSetAtomId(mDelegate->mEvent, - android::util::SURFACEFLINGER_STATS_LAYER_INFO)); - EXPECT_CALL(*mDelegate, - statsEventWriteString8(mDelegate->mEvent, - StrEq(genLayerName(LAYER_ID_0).c_str()))); - EXPECT_CALL(*mDelegate, statsEventWriteInt64(mDelegate->mEvent, 1)); - EXPECT_CALL(*mDelegate, statsEventWriteInt64(mDelegate->mEvent, 0)); - EXPECT_CALL(*mDelegate, - statsEventWriteByteArray(mDelegate->mEvent, - BytesEq((const uint8_t*) - expectedPresentToPresent.c_str(), - expectedPresentToPresent.size()), - expectedPresentToPresent.size())); - EXPECT_CALL(*mDelegate, - statsEventWriteByteArray(mDelegate->mEvent, - BytesEq((const uint8_t*)expectedPostToPresent.c_str(), - expectedPostToPresent.size()), - expectedPostToPresent.size())); - EXPECT_CALL(*mDelegate, - statsEventWriteByteArray(mDelegate->mEvent, - BytesEq((const uint8_t*) - expectedAcquireToPresent.c_str(), - expectedAcquireToPresent.size()), - expectedAcquireToPresent.size())); - EXPECT_CALL(*mDelegate, - statsEventWriteByteArray(mDelegate->mEvent, - BytesEq((const uint8_t*)expectedLatchToPresent.c_str(), - expectedLatchToPresent.size()), - expectedLatchToPresent.size())); - EXPECT_CALL(*mDelegate, - statsEventWriteByteArray(mDelegate->mEvent, - BytesEq((const uint8_t*) - expectedDesiredToPresent.c_str(), - expectedDesiredToPresent.size()), - expectedDesiredToPresent.size())); - EXPECT_CALL(*mDelegate, - statsEventWriteByteArray(mDelegate->mEvent, - BytesEq((const uint8_t*)expectedPostToAcquire.c_str(), - expectedPostToAcquire.size()), - expectedPostToAcquire.size())); - EXPECT_CALL(*mDelegate, statsEventWriteInt64(mDelegate->mEvent, LATE_ACQUIRE_FRAMES)); - EXPECT_CALL(*mDelegate, - statsEventWriteInt64(mDelegate->mEvent, BAD_DESIRED_PRESENT_FRAMES)); - EXPECT_CALL(*mDelegate, statsEventWriteInt32(mDelegate->mEvent, UID_0)); - EXPECT_CALL(*mDelegate, statsEventWriteInt32(mDelegate->mEvent, 8)); - EXPECT_CALL(*mDelegate, statsEventWriteInt32(mDelegate->mEvent, 7)); - EXPECT_CALL(*mDelegate, statsEventWriteInt32(mDelegate->mEvent, 1)); - EXPECT_CALL(*mDelegate, statsEventWriteInt32(mDelegate->mEvent, 1)); - EXPECT_CALL(*mDelegate, statsEventWriteInt32(mDelegate->mEvent, 1)); - EXPECT_CALL(*mDelegate, statsEventWriteInt32(mDelegate->mEvent, 2)); - EXPECT_CALL(*mDelegate, statsEventWriteInt32(mDelegate->mEvent, 1)); - EXPECT_CALL(*mDelegate, statsEventWriteInt32(mDelegate->mEvent, 1)); - EXPECT_CALL(*mDelegate, statsEventWriteInt32(mDelegate->mEvent, 1)); - EXPECT_CALL(*mDelegate, statsEventWriteInt32(mDelegate->mEvent, REFRESH_RATE_BUCKET_0)); - EXPECT_CALL(*mDelegate, statsEventWriteInt32(mDelegate->mEvent, RENDER_RATE_BUCKET_0)); - EXPECT_CALL(*mDelegate, - statsEventWriteByteArray(mDelegate->mEvent, - BytesEq((const uint8_t*) - expectedFrameRateOverride.c_str(), - expectedFrameRateOverride.size()), - expectedFrameRateOverride.size())); - EXPECT_CALL(*mDelegate, - statsEventWriteByteArray(mDelegate->mEvent, - BytesEq((const uint8_t*) - expectedAppDeadlineMissed.c_str(), - expectedAppDeadlineMissed.size()), - expectedAppDeadlineMissed.size())); - - EXPECT_CALL(*mDelegate, statsEventBuild(mDelegate->mEvent)); - } - EXPECT_EQ(AStatsManager_PULL_SUCCESS, - mDelegate->makePullAtomCallback(android::util::SURFACEFLINGER_STATS_LAYER_INFO, - mDelegate->mCookie)); + std::string pulledData; + EXPECT_TRUE(mTimeStats->onPullAtom(10063 /*SURFACEFLINGER_STATS_LAYER_INFO*/, &pulledData)); + + android::surfaceflinger::SurfaceflingerStatsLayerInfoWrapper atomList; + ASSERT_TRUE(atomList.ParseFromString(pulledData)); + ASSERT_EQ(atomList.atom_size(), 1); + const android::surfaceflinger::SurfaceflingerStatsLayerInfo& atom = atomList.atom(0); + + EXPECT_EQ(atom.layer_name(), genLayerName(LAYER_ID_0)); + EXPECT_EQ(atom.total_frames(), 1); + EXPECT_EQ(atom.dropped_frames(), 0); + EXPECT_THAT(atom.present_to_present(), HistogramEq(buildExpectedHistogram({1}, {1}))); + EXPECT_THAT(atom.post_to_present(), HistogramEq(buildExpectedHistogram({4}, {1}))); + EXPECT_THAT(atom.acquire_to_present(), HistogramEq(buildExpectedHistogram({3}, {1}))); + EXPECT_THAT(atom.latch_to_present(), HistogramEq(buildExpectedHistogram({2}, {1}))); + EXPECT_THAT(atom.desired_to_present(), HistogramEq(buildExpectedHistogram({1}, {1}))); + EXPECT_THAT(atom.post_to_acquire(), HistogramEq(buildExpectedHistogram({1}, {1}))); + EXPECT_EQ(atom.late_acquire_frames(), LATE_ACQUIRE_FRAMES); + EXPECT_EQ(atom.bad_desired_present_frames(), BAD_DESIRED_PRESENT_FRAMES); + EXPECT_EQ(atom.uid(), UID_0); + EXPECT_EQ(atom.total_timeline_frames(), 8); + EXPECT_EQ(atom.total_janky_frames(), 7); + EXPECT_EQ(atom.total_janky_frames_with_long_cpu(), 1); + EXPECT_EQ(atom.total_janky_frames_with_long_gpu(), 1); + EXPECT_EQ(atom.total_janky_frames_sf_unattributed(), 1); + EXPECT_EQ(atom.total_janky_frames_app_unattributed(), 2); + EXPECT_EQ(atom.total_janky_frames_sf_scheduling(), 1); + EXPECT_EQ(atom.total_jank_frames_sf_prediction_error(), 1); + EXPECT_EQ(atom.total_jank_frames_app_buffer_stuffing(), 1); + EXPECT_EQ(atom.display_refresh_rate_bucket(), REFRESH_RATE_BUCKET_0); + EXPECT_EQ(atom.render_rate_bucket(), RENDER_RATE_BUCKET_0); + EXPECT_THAT(atom.set_frame_rate_vote().frame_rate(), testing::FloatEq(frameRate60.frameRate)); + EXPECT_EQ((int)atom.set_frame_rate_vote().frame_rate_compatibility(), + (int)frameRate60.frameRateCompatibility); + EXPECT_EQ((int)atom.set_frame_rate_vote().seamlessness(), (int)frameRate60.seamlessness); + EXPECT_THAT(atom.app_deadline_misses(), HistogramEq(buildExpectedHistogram({3, 2}, {4, 3}))); SFTimeStatsGlobalProto globalProto; ASSERT_TRUE(globalProto.ParseFromString(inputCommand(InputCommand::DUMP_ALL, FMT_PROTO))); @@ -1398,37 +1243,26 @@ TEST_F(TimeStatsTest, layerStatsCallback_pullsAllAndClears) { TEST_F(TimeStatsTest, layerStatsCallback_pullsMultipleLayers) { EXPECT_TRUE(inputCommand(InputCommand::ENABLE, FMT_STRING).empty()); - mTimeStats->onBootFinished(); - insertTimeRecord(NORMAL_SEQUENCE, LAYER_ID_0, 1, 1000000); insertTimeRecord(NORMAL_SEQUENCE, LAYER_ID_0, 2, 2000000); insertTimeRecord(NORMAL_SEQUENCE, LAYER_ID_1, 1, 2000000); insertTimeRecord(NORMAL_SEQUENCE, LAYER_ID_1, 2, 3000000); - EXPECT_THAT(mDelegate->mAtomTags, - UnorderedElementsAre(android::util::SURFACEFLINGER_STATS_GLOBAL_INFO, - android::util::SURFACEFLINGER_STATS_LAYER_INFO)); - EXPECT_NE(nullptr, mDelegate->mCallback); - EXPECT_EQ(mTimeStats.get(), mDelegate->mCookie); - - EXPECT_CALL(*mDelegate, - statsEventSetAtomId(mDelegate->mEvent, - android::util::SURFACEFLINGER_STATS_LAYER_INFO)) - .Times(2); - EXPECT_CALL(*mDelegate, - statsEventWriteString8(mDelegate->mEvent, StrEq(genLayerName(LAYER_ID_0).c_str()))); - EXPECT_CALL(*mDelegate, - statsEventWriteString8(mDelegate->mEvent, StrEq(genLayerName(LAYER_ID_1).c_str()))); - EXPECT_EQ(AStatsManager_PULL_SUCCESS, - mDelegate->makePullAtomCallback(android::util::SURFACEFLINGER_STATS_LAYER_INFO, - mDelegate->mCookie)); + std::string pulledData; + EXPECT_TRUE(mTimeStats->onPullAtom(10063 /*SURFACEFLINGER_STATS_LAYER_INFO*/, &pulledData)); + + android::surfaceflinger::SurfaceflingerStatsLayerInfoWrapper atomList; + ASSERT_TRUE(atomList.ParseFromString(pulledData)); + ASSERT_EQ(atomList.atom_size(), 2); + std::vector<std::string> actualLayerNames = {atomList.atom(0).layer_name(), + atomList.atom(1).layer_name()}; + EXPECT_THAT(actualLayerNames, + UnorderedElementsAre(genLayerName(LAYER_ID_0), genLayerName(LAYER_ID_1))); } TEST_F(TimeStatsTest, layerStatsCallback_pullsMultipleBuckets) { EXPECT_TRUE(inputCommand(InputCommand::ENABLE, FMT_STRING).empty()); - mTimeStats->onBootFinished(); - insertTimeRecord(NORMAL_SEQUENCE, LAYER_ID_0, 1, 1000000); insertTimeRecord(NORMAL_SEQUENCE, LAYER_ID_0, 2, 2000000); insertTimeRecord(NORMAL_SEQUENCE, LAYER_ID_0, 3, 4000000); @@ -1438,102 +1272,53 @@ TEST_F(TimeStatsTest, layerStatsCallback_pullsMultipleBuckets) { mTimeStats->setPowerMode(PowerMode::ON); mTimeStats->setPresentFenceGlobal(std::make_shared<FenceTime>(3000000)); mTimeStats->setPresentFenceGlobal(std::make_shared<FenceTime>(5000000)); - EXPECT_THAT(mDelegate->mAtomTags, - UnorderedElementsAre(android::util::SURFACEFLINGER_STATS_GLOBAL_INFO, - android::util::SURFACEFLINGER_STATS_LAYER_INFO)); - EXPECT_NE(nullptr, mDelegate->mCallback); - EXPECT_EQ(mTimeStats.get(), mDelegate->mCookie); - - EXPECT_THAT(mDelegate->mAtomTags, - UnorderedElementsAre(android::util::SURFACEFLINGER_STATS_GLOBAL_INFO, - android::util::SURFACEFLINGER_STATS_LAYER_INFO)); - std::string expectedPresentToPresent = buildExpectedHistogramBytestring({1, 2}, {2, 1}); - { - InSequence seq; - EXPECT_CALL(*mDelegate, - statsEventWriteByteArray(mDelegate->mEvent, - BytesEq((const uint8_t*) - expectedPresentToPresent.c_str(), - expectedPresentToPresent.size()), - expectedPresentToPresent.size())); - EXPECT_CALL(*mDelegate, statsEventWriteByteArray(mDelegate->mEvent, _, _)) - .Times(AnyNumber()); - } - EXPECT_EQ(AStatsManager_PULL_SUCCESS, - mDelegate->makePullAtomCallback(android::util::SURFACEFLINGER_STATS_LAYER_INFO, - mDelegate->mCookie)); + + std::string pulledData; + EXPECT_TRUE(mTimeStats->onPullAtom(10063 /*SURFACEFLINGER_STATS_LAYER_INFO*/, &pulledData)); + + android::surfaceflinger::SurfaceflingerStatsLayerInfoWrapper atomList; + ASSERT_TRUE(atomList.ParseFromString(pulledData)); + ASSERT_EQ(atomList.atom_size(), 1); + const android::surfaceflinger::SurfaceflingerStatsLayerInfo& atom = atomList.atom(0); + EXPECT_THAT(atom.present_to_present(), HistogramEq(buildExpectedHistogram({1, 2}, {2, 1}))); } TEST_F(TimeStatsTest, layerStatsCallback_limitsHistogramBuckets) { - mDelegate = new FakeStatsEventDelegate; - mTimeStats = - std::make_unique<impl::TimeStats>(std::unique_ptr<FakeStatsEventDelegate>(mDelegate), - std::nullopt, 1); + mTimeStats = std::make_unique<impl::TimeStats>(std::nullopt, 1); EXPECT_TRUE(inputCommand(InputCommand::ENABLE, FMT_STRING).empty()); - mTimeStats->onBootFinished(); - insertTimeRecord(NORMAL_SEQUENCE, LAYER_ID_0, 1, 1000000); insertTimeRecord(NORMAL_SEQUENCE, LAYER_ID_0, 2, 2000000); insertTimeRecord(NORMAL_SEQUENCE, LAYER_ID_0, 3, 4000000); insertTimeRecord(NORMAL_SEQUENCE, LAYER_ID_0, 4, 5000000); - EXPECT_THAT(mDelegate->mAtomTags, - UnorderedElementsAre(android::util::SURFACEFLINGER_STATS_GLOBAL_INFO, - android::util::SURFACEFLINGER_STATS_LAYER_INFO)); - EXPECT_NE(nullptr, mDelegate->mCallback); - EXPECT_EQ(mTimeStats.get(), mDelegate->mCookie); - - EXPECT_THAT(mDelegate->mAtomTags, - UnorderedElementsAre(android::util::SURFACEFLINGER_STATS_GLOBAL_INFO, - android::util::SURFACEFLINGER_STATS_LAYER_INFO)); - std::string expectedPresentToPresent = buildExpectedHistogramBytestring({1}, {2}); - { - InSequence seq; - EXPECT_CALL(*mDelegate, - statsEventWriteByteArray(mDelegate->mEvent, - BytesEq((const uint8_t*) - expectedPresentToPresent.c_str(), - expectedPresentToPresent.size()), - expectedPresentToPresent.size())); - EXPECT_CALL(*mDelegate, statsEventWriteByteArray(mDelegate->mEvent, _, _)) - .Times(AnyNumber()); - } - EXPECT_EQ(AStatsManager_PULL_SUCCESS, - mDelegate->makePullAtomCallback(android::util::SURFACEFLINGER_STATS_LAYER_INFO, - mDelegate->mCookie)); + std::string pulledData; + EXPECT_TRUE(mTimeStats->onPullAtom(10063 /*SURFACEFLINGER_STATS_LAYER_INFO*/, &pulledData)); + + android::surfaceflinger::SurfaceflingerStatsLayerInfoWrapper atomList; + ASSERT_TRUE(atomList.ParseFromString(pulledData)); + ASSERT_EQ(atomList.atom_size(), 1); + const android::surfaceflinger::SurfaceflingerStatsLayerInfo& atom = atomList.atom(0); + EXPECT_THAT(atom.present_to_present(), HistogramEq(buildExpectedHistogram({1}, {2}))); } TEST_F(TimeStatsTest, layerStatsCallback_limitsLayers) { - mDelegate = new FakeStatsEventDelegate; - mTimeStats = - std::make_unique<impl::TimeStats>(std::unique_ptr<FakeStatsEventDelegate>(mDelegate), 1, - std::nullopt); + mTimeStats = std::make_unique<impl::TimeStats>(1, std::nullopt); EXPECT_TRUE(inputCommand(InputCommand::ENABLE, FMT_STRING).empty()); - mTimeStats->onBootFinished(); - insertTimeRecord(NORMAL_SEQUENCE, LAYER_ID_0, 1, 1000000); insertTimeRecord(NORMAL_SEQUENCE, LAYER_ID_0, 2, 2000000); insertTimeRecord(NORMAL_SEQUENCE, LAYER_ID_1, 1, 2000000); insertTimeRecord(NORMAL_SEQUENCE, LAYER_ID_1, 2, 3000000); insertTimeRecord(NORMAL_SEQUENCE, LAYER_ID_1, 4, 5000000); - EXPECT_THAT(mDelegate->mAtomTags, - UnorderedElementsAre(android::util::SURFACEFLINGER_STATS_GLOBAL_INFO, - android::util::SURFACEFLINGER_STATS_LAYER_INFO)); - EXPECT_NE(nullptr, mDelegate->mCallback); - EXPECT_EQ(mTimeStats.get(), mDelegate->mCookie); - - EXPECT_CALL(*mDelegate, - statsEventSetAtomId(mDelegate->mEvent, - android::util::SURFACEFLINGER_STATS_LAYER_INFO)) - .Times(1); - EXPECT_CALL(*mDelegate, - statsEventWriteString8(mDelegate->mEvent, StrEq(genLayerName(LAYER_ID_1).c_str()))); - EXPECT_EQ(AStatsManager_PULL_SUCCESS, - mDelegate->makePullAtomCallback(android::util::SURFACEFLINGER_STATS_LAYER_INFO, - mDelegate->mCookie)); + std::string pulledData; + EXPECT_TRUE(mTimeStats->onPullAtom(10063 /*SURFACEFLINGER_STATS_LAYER_INFO*/, &pulledData)); + + android::surfaceflinger::SurfaceflingerStatsLayerInfoWrapper atomList; + ASSERT_TRUE(atomList.ParseFromString(pulledData)); + ASSERT_EQ(atomList.atom_size(), 1); + EXPECT_EQ(atomList.atom(0).layer_name(), genLayerName(LAYER_ID_1)); } TEST_F(TimeStatsTest, canSurviveMonkey) { diff --git a/services/surfaceflinger/tests/unittests/mock/MockTimeStats.h b/services/surfaceflinger/tests/unittests/mock/MockTimeStats.h index 37b74ed3a7..526a847614 100644 --- a/services/surfaceflinger/tests/unittests/mock/MockTimeStats.h +++ b/services/surfaceflinger/tests/unittests/mock/MockTimeStats.h @@ -27,7 +27,7 @@ public: TimeStats(); ~TimeStats() override; - MOCK_METHOD0(onBootFinished, void()); + MOCK_METHOD2(onPullAtom, bool(const int, std::string*)); MOCK_METHOD3(parseArgs, void(bool, const Vector<String16>&, std::string&)); MOCK_METHOD0(isEnabled, bool()); MOCK_METHOD0(miniDump, std::string()); |