Merge "Fix cursor event would be dropped" into sc-dev
diff --git a/cmds/dumpsys/OWNERS b/cmds/dumpsys/OWNERS
index 2a9b681..4f6a89e 100644
--- a/cmds/dumpsys/OWNERS
+++ b/cmds/dumpsys/OWNERS
@@ -2,3 +2,6 @@
nandana@google.com
jsharkey@android.com
+
+# for ServiceManager mock
+per-file dumpsys_test.cpp=smoreland@google.com
diff --git a/cmds/dumpsys/tests/dumpsys_test.cpp b/cmds/dumpsys/tests/dumpsys_test.cpp
index 39af7df..c9d2dbb 100644
--- a/cmds/dumpsys/tests/dumpsys_test.cpp
+++ b/cmds/dumpsys/tests/dumpsys_test.cpp
@@ -59,6 +59,7 @@
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*());
};
diff --git a/cmds/servicemanager/ServiceManager.cpp b/cmds/servicemanager/ServiceManager.cpp
index 2f55249..b429fb3 100644
--- a/cmds/servicemanager/ServiceManager.cpp
+++ b/cmds/servicemanager/ServiceManager.cpp
@@ -58,22 +58,34 @@
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;
- }
- 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);
+struct AidlName {
+ std::string package;
+ std::string iface;
+ std::string instance;
- bool found = forEachManifest([&] (const ManifestWithDescription& mwd) {
- if (mwd.manifest->hasAidlInstance(package, iface, 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;
+ }
+};
+
+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(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 @@
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 @@
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 c089115..4f23c21 100644
--- a/cmds/servicemanager/ServiceManager.h
+++ b/cmds/servicemanager/ServiceManager.h
@@ -46,6 +46,8 @@
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 c349024..b7eafcd 100644
--- a/include/android/surface_control.h
+++ b/include/android/surface_control.h
@@ -147,6 +147,28 @@
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 @@
* 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 @@
* 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 @@
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.
*
diff --git a/libs/binder/IPCThreadState.cpp b/libs/binder/IPCThreadState.cpp
index 406bd54..6fb1227 100644
--- a/libs/binder/IPCThreadState.cpp
+++ b/libs/binder/IPCThreadState.cpp
@@ -90,6 +90,8 @@
"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 @@
}
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 61f4581..f684cf6 100644
--- a/libs/binder/IServiceManager.cpp
+++ b/libs/binder/IServiceManager.cpp
@@ -75,6 +75,7 @@
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 @@
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 7647a8c..0414e76 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 @@
return result;
}
+status_t ProcessState::enableOnewaySpamDetection(bool enable) {
+ uint32_t enableDetection = enable ? 1 : 0;
+ if (ioctl(mDriverFD, BINDER_ENABLE_ONEWAY_SPAM_DETECTION, &enableDetection) == -1) {
+ ALOGE("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 @@
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) {
+ ALOGE("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/aidl/android/os/IServiceManager.aidl b/libs/binder/aidl/android/os/IServiceManager.aidl
index 2fabf94..75c4092 100644
--- a/libs/binder/aidl/android/os/IServiceManager.aidl
+++ b/libs/binder/aidl/android/os/IServiceManager.aidl
@@ -108,6 +108,11 @@
@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 5f0d056..3dbe2c4 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 @@
* 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 0919648..b9db5d7 100644
--- a/libs/binder/include/binder/ProcessState.h
+++ b/libs/binder/include/binder/ProcessState.h
@@ -58,6 +58,7 @@
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/private/binder/binder_module.h b/libs/binder/include/private/binder/binder_module.h
index 1579199..151235c 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 @@
};
#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 5516914..a90b4aa 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 @@
__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 67c85b6..7d4b82e 100644
--- a/libs/binder/ndk/libbinder_ndk.map.txt
+++ b/libs/binder/ndk/libbinder_ndk.map.txt
@@ -118,14 +118,15 @@
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 1ccd0d2..7649a26 100644
--- a/libs/binder/ndk/service_manager.cpp
+++ b/libs/binder/ndk/service_manager.cpp
@@ -105,6 +105,14 @@
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 6a88401..62db3cf 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 @@
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/tests/binderLibTest.cpp b/libs/binder/tests/binderLibTest.cpp
index e2193fa..dc8c0f1 100644
--- a/libs/binder/tests/binderLibTest.cpp
+++ b/libs/binder/tests/binderLibTest.cpp
@@ -88,6 +88,7 @@
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 @@
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 @@
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 ec4ced2..dd68fdb 100644
--- a/libs/binder/tests/binderRpcTest.cpp
+++ b/libs/binder/tests/binderRpcTest.cpp
@@ -80,11 +80,10 @@
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();
}
@@ -749,7 +748,7 @@
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);
}
}));
@@ -758,6 +757,28 @@
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;
diff --git a/libs/binder/tests/binderStabilityTest.cpp b/libs/binder/tests/binderStabilityTest.cpp
index cb309bd..2ce13df 100644
--- a/libs/binder/tests/binderStabilityTest.cpp
+++ b/libs/binder/tests/binderStabilityTest.cpp
@@ -192,6 +192,8 @@
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 4ecbe53..761e45c 100644
--- a/libs/fakeservicemanager/ServiceManager.cpp
+++ b/libs/fakeservicemanager/ServiceManager.cpp
@@ -73,4 +73,9 @@
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 4ef47fb..e26c21b 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 @@
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 dbc1a7e..3d854c2 100644
--- a/libs/gui/BLASTBufferQueue.cpp
+++ b/libs/gui/BLASTBufferQueue.cpp
@@ -186,6 +186,7 @@
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 @@
// 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 @@
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 @@
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 6ca2487..53721cf 100644
--- a/libs/gui/ISurfaceComposer.cpp
+++ b/libs/gui/ISurfaceComposer.cpp
@@ -99,7 +99,7 @@
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);
@@ -1268,7 +1268,7 @@
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);
}
diff --git a/libs/gui/ITransactionCompletedListener.cpp b/libs/gui/ITransactionCompletedListener.cpp
index b42793b..f74f91e 100644
--- a/libs/gui/ITransactionCompletedListener.cpp
+++ b/libs/gui/ITransactionCompletedListener.cpp
@@ -152,7 +152,7 @@
}
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::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 @@
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 @@
}
}
+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 55ed7fe..517b49e 100644
--- a/libs/gui/LayerState.cpp
+++ b/libs/gui/LayerState.cpp
@@ -140,7 +140,7 @@
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 @@
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 5dc2907..5db0eae 100644
--- a/libs/gui/SurfaceComposerClient.cpp
+++ b/libs/gui/SurfaceComposerClient.cpp
@@ -139,7 +139,7 @@
// 0 is an invalid callback id
TransactionCompletedListener::TransactionCompletedListener() : mCallbackIdCounter(1) {}
-CallbackId TransactionCompletedListener::getNextIdLocked() {
+int64_t TransactionCompletedListener::getNextIdLocked() {
return mCallbackIdCounter++;
}
@@ -163,13 +163,13 @@
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::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::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 @@
}
}
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 @@
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 @@
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 @@
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 @@
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);
diff --git a/libs/gui/include/gui/BLASTBufferQueue.h b/libs/gui/include/gui/BLASTBufferQueue.h
index 8a23223..139dbb7 100644
--- a/libs/gui/include/gui/BLASTBufferQueue.h
+++ b/libs/gui/include/gui/BLASTBufferQueue.h
@@ -151,14 +151,20 @@
// 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/ITransactionCompletedListener.h b/libs/gui/include/gui/ITransactionCompletedListener.h
index 098760e..2d71194 100644
--- a/libs/gui/include/gui/ITransactionCompletedListener.h
+++ b/libs/gui/include/gui/ITransactionCompletedListener.h
@@ -36,7 +36,22 @@
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 @@
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 @@
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 @@
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 @@
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 @@
// 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 d2d1e5b..b4f62f2 100644
--- a/libs/gui/include/gui/LayerState.h
+++ b/libs/gui/include/gui/LayerState.h
@@ -84,6 +84,7 @@
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 d3f6ff0..5bbd8e3 100644
--- a/libs/gui/include/gui/SurfaceComposerClient.h
+++ b/libs/gui/include/gui/SurfaceComposerClient.h
@@ -336,7 +336,7 @@
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 @@
// 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.
@@ -621,14 +627,13 @@
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>
@@ -646,7 +651,8 @@
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);
@@ -662,10 +668,12 @@
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 a44f44f..5a5da97 100644
--- a/libs/gui/tests/BLASTBufferQueue_test.cpp
+++ b/libs/gui/tests/BLASTBufferQueue_test.cpp
@@ -522,16 +522,146 @@
adapter.waitForCallbacks();
// capture screen and verify that it is red
ASSERT_EQ(NO_ERROR, captureDisplay(mCaptureArgs, mCaptureResults));
-
- Rect bounds;
- bounds.left = finalCropSideLength / 2;
- bounds.top = 0;
- bounds.right = bounds.left + finalCropSideLength;
- bounds.bottom = finalCropSideLength;
-
- ASSERT_NO_FATAL_FAILURE(checkScreenCapture(r, g, b, bounds));
+ 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, bounds, /*border*/ 0, /*outsideRegion*/ true));
+ checkScreenCapture(0, 0, 0,
+ {0, 0, (int32_t)bufferSideLength, (int32_t)bufferSideLength},
+ /*border*/ 0, /*outsideRegion*/ true));
+}
+
+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();
+
+ 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 4c208b3..ea8c295 100644
--- a/libs/gui/tests/Surface_test.cpp
+++ b/libs/gui/tests/Surface_test.cpp
@@ -102,14 +102,13 @@
// 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);
diff --git a/libs/permission/Android.bp b/libs/permission/Android.bp
index dd38224..a5712b3 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/skia/AutoBackendTexture.cpp b/libs/renderengine/skia/AutoBackendTexture.cpp
index c535597..9ed759f 100644
--- a/libs/renderengine/skia/AutoBackendTexture.cpp
+++ b/libs/renderengine/skia/AutoBackendTexture.cpp
@@ -29,7 +29,8 @@
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 @@
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 @@
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 @@
}
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 2d61cf8..3133de6 100644
--- a/libs/renderengine/skia/AutoBackendTexture.h
+++ b/libs/renderengine/skia/AutoBackendTexture.h
@@ -41,34 +41,42 @@
// of shared ownership with Skia objects, so we wrap it here instead.
class LocalRef {
public:
- LocalRef(AutoBackendTexture* texture) { setTexture(texture); }
-
- ~LocalRef() {
- // Destroying the texture is the same as setting it to null
- setTexture(nullptr);
+ LocalRef(GrDirectContext* context, AHardwareBuffer* buffer, bool isOutputBuffer) {
+ mTexture = new AutoBackendTexture(context, buffer, isOutputBuffer);
+ mTexture->ref();
}
- AutoBackendTexture* getTexture() const { return mTexture; }
+ ~LocalRef() {
+ if (mTexture != nullptr) {
+ mTexture->unref(true);
+ }
+ }
+
+ // 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 @@
// 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 @@
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 37d98a3..0a84754 100644
--- a/libs/renderengine/skia/SkiaGLRenderEngine.cpp
+++ b/libs/renderengine/skia/SkiaGLRenderEngine.cpp
@@ -508,9 +508,9 @@
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 @@
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 @@
// 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 d63c88b..34ef0a4 100644
--- a/libs/renderengine/tests/RenderEngineTest.cpp
+++ b/libs/renderengine/tests/RenderEngineTest.cpp
@@ -1402,7 +1402,8 @@
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 @@
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 @@
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/services/surfaceflinger/BufferStateLayer.cpp b/services/surfaceflinger/BufferStateLayer.cpp
index ac2edbe..7a5b20d 100644
--- a/services/surfaceflinger/BufferStateLayer.cpp
+++ b/services/surfaceflinger/BufferStateLayer.cpp
@@ -674,6 +674,11 @@
latchTime);
}
+ std::deque<sp<CallbackHandle>> remainingHandles;
+ mFlinger->getTransactionCallbackInvoker()
+ .finalizeOnCommitCallbackHandles(mDrawingState.callbackHandles, remainingHandles);
+ mDrawingState.callbackHandles = remainingHandles;
+
mCurrentStateModified = false;
return NO_ERROR;
@@ -790,7 +795,7 @@
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 @@
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 4171092..af4fcae 100644
--- a/services/surfaceflinger/BufferStateLayer.h
+++ b/services/surfaceflinger/BufferStateLayer.h
@@ -140,7 +140,7 @@
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 a0606b4..289cb11 100644
--- a/services/surfaceflinger/CompositionEngine/include/compositionengine/CompositionRefreshArgs.h
+++ b/services/surfaceflinger/CompositionEngine/include/compositionengine/CompositionRefreshArgs.h
@@ -79,6 +79,9 @@
// 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/impl/OutputCompositionState.h b/services/surfaceflinger/CompositionEngine/include/compositionengine/impl/OutputCompositionState.h
index 8f767d3..f0ef6d6 100644
--- a/services/surfaceflinger/CompositionEngine/include/compositionengine/impl/OutputCompositionState.h
+++ b/services/surfaceflinger/CompositionEngine/include/compositionengine/impl/OutputCompositionState.h
@@ -115,6 +115,9 @@
// 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/src/Display.cpp b/services/surfaceflinger/CompositionEngine/src/Display.cpp
index a605fe1..1ffb1c8 100644
--- a/services/surfaceflinger/CompositionEngine/src/Display.cpp
+++ b/services/surfaceflinger/CompositionEngine/src/Display.cpp
@@ -367,6 +367,11 @@
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 3468b20..faa4b74 100644
--- a/services/surfaceflinger/CompositionEngine/src/Output.cpp
+++ b/services/surfaceflinger/CompositionEngine/src/Output.cpp
@@ -711,6 +711,8 @@
return;
}
+ editState().earliestPresentTime = refreshArgs.earliestPresentTime;
+
sp<GraphicBuffer> previousOverride = nullptr;
for (auto* layer : getOutputLayersOrderedByZ()) {
bool skipLayer = false;
diff --git a/services/surfaceflinger/Scheduler/OneShotTimer.cpp b/services/surfaceflinger/Scheduler/OneShotTimer.cpp
index ce3b0c6..d659398 100644
--- a/services/surfaceflinger/Scheduler/OneShotTimer.cpp
+++ b/services/surfaceflinger/Scheduler/OneShotTimer.cpp
@@ -40,13 +40,21 @@
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 @@
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 @@
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 3690ce7..7285427 100644
--- a/services/surfaceflinger/Scheduler/OneShotTimer.h
+++ b/services/surfaceflinger/Scheduler/OneShotTimer.h
@@ -36,8 +36,17 @@
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 @@
// 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 1d25c72..57bd045 100644
--- a/services/surfaceflinger/Scheduler/Scheduler.cpp
+++ b/services/surfaceflinger/Scheduler/Scheduler.cpp
@@ -927,4 +927,11 @@
}
}
+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 4d1f3c6..49d3d93 100644
--- a/services/surfaceflinger/Scheduler/Scheduler.h
+++ b/services/surfaceflinger/Scheduler/Scheduler.h
@@ -149,6 +149,8 @@
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 7076124..2da8ed4 100644
--- a/services/surfaceflinger/SurfaceFlinger.cpp
+++ b/services/surfaceflinger/SurfaceFlinger.cpp
@@ -1972,6 +1972,8 @@
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
@@ -2032,6 +2034,9 @@
ATRACE_CALL();
bool refreshNeeded = handlePageFlip();
+ // Send on commit callbacks
+ mTransactionCallbackInvoker.sendCallbacks();
+
if (mVisibleRegionsDirty) {
computeLayerBounds();
}
@@ -2754,6 +2759,7 @@
(currentState.orientedDisplaySpaceRect != drawingState.orientedDisplaySpaceRect)) {
display->setProjection(currentState.orientation, currentState.layerStackSpaceRect,
currentState.orientedDisplaySpaceRect);
+ mDefaultDisplayTransformHint = display->getTransformHint();
}
if (currentState.width != drawingState.width ||
currentState.height != drawingState.height) {
@@ -3260,70 +3266,38 @@
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;
- }
-
- mLayersByLocalBinderToken.emplace(handle->localBinder(), lbc);
-
- 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());
- }
- }
-
- 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();
- }
-
- mLayersAdded = true;
+ if (mNumLayers >= ISurfaceComposer::MAX_LAYERS) {
+ ALOGE("AddClientLayer failed, mNumLayers (%zu) >= MAX_LAYERS (%zu)", mNumLayers.load(),
+ ISurfaceComposer::MAX_LAYERS);
+ return NO_MEMORY;
}
+ wp<IBinder> initialProducer;
+ if (gbc != nullptr) {
+ initialProducer = IInterface::asBinder(gbc);
+ }
+ setLayerCreatedState(handle, lbc, parentHandle, parentLayer, initialProducer);
+
+ // Create a transaction includes the initial parent and producer.
+ Vector<ComposerState> states;
+ Vector<DisplayState> displays;
+
+ ComposerState composerState;
+ composerState.state.what = layer_state_t::eLayerCreated;
+ composerState.state.surface = handle;
+ states.add(composerState);
+
+ 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) {
@@ -3792,19 +3766,47 @@
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");
@@ -3817,10 +3819,6 @@
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();
@@ -4052,8 +4050,8 @@
}
}
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));
}
}
@@ -4274,14 +4272,7 @@
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;
@@ -4369,7 +4360,7 @@
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};
@@ -6632,6 +6623,87 @@
}
}
+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 fa503c3..cf1a545 100644
--- a/services/surfaceflinger/SurfaceFlinger.h
+++ b/services/surfaceflinger/SurfaceFlinger.h
@@ -1392,6 +1392,35 @@
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/TimeStats.cpp b/services/surfaceflinger/TimeStats/TimeStats.cpp
index 16c6bb3..3d82afa 100644
--- a/services/surfaceflinger/TimeStats/TimeStats.cpp
+++ b/services/surfaceflinger/TimeStats/TimeStats.cpp
@@ -745,7 +745,7 @@
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) {
@@ -771,9 +771,11 @@
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/TransactionCallbackInvoker.cpp b/services/surfaceflinger/TransactionCallbackInvoker.cpp
index 3590e76..4f4c02b 100644
--- a/services/surfaceflinger/TransactionCallbackInvoker.cpp
+++ b/services/surfaceflinger/TransactionCallbackInvoker.cpp
@@ -36,13 +36,17 @@
// <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 @@
return NO_ERROR;
}
+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) {
+ if (!containsOnCommitCallbacks(handle->callbackIds)) {
+ outRemainingHandles.push_back(handle);
+ continue;
+ }
+ status_t err = finalizeCallbackHandle(handle, jankData);
+ if (err != NO_ERROR) {
+ return err;
+ }
+ }
+
+ 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) {
- 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);
+ status_t err = finalizeCallbackHandle(handle, jankData);
if (err != NO_ERROR) {
- ALOGE("could not add callback handle");
return err;
}
}
@@ -243,7 +277,8 @@
}
// 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 caa8a4f..184b151 100644
--- a/services/surfaceflinger/TransactionCallbackInvoker.h
+++ b/services/surfaceflinger/TransactionCallbackInvoker.h
@@ -74,6 +74,8 @@
// 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 @@
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 58b039e..9cf7c09 100644
--- a/services/surfaceflinger/tests/InvalidHandles_test.cpp
+++ b/services/surfaceflinger/tests/InvalidHandles_test.cpp
@@ -52,12 +52,15 @@
}
};
-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/OneShotTimerTest.cpp b/services/surfaceflinger/tests/unittests/OneShotTimerTest.cpp
index cfbb3f5..a1f0588 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 @@
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 @@
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);
+ EXPECT_FALSE(mExpiredTimerCallback.waitForUnexpectedCall().has_value());
- std::this_thread::sleep_for(std::chrono::milliseconds(25));
- EXPECT_FALSE(mResetTimerCallback.waitForCall().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());
- // A single callback should be generated after 30ms
- EXPECT_TRUE(
- mExpiredTimerCallback.waitForCall(waitTimeForExpected3msCallback + 30ms).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, 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());
- // 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
+ EXPECT_FALSE(mExpiredTimerCallback.waitForUnexpectedCall().has_value());
+ EXPECT_FALSE(mResetTimerCallback.waitForUnexpectedCall().has_value());
+
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 4e73cbc..188ea75 100644
--- a/services/surfaceflinger/tests/unittests/TimeStatsTest.cpp
+++ b/services/surfaceflinger/tests/unittests/TimeStatsTest.cpp
@@ -1051,6 +1051,8 @@
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});
std::string pulledData;
@@ -1069,7 +1071,7 @@
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(), 8);
+ 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);
@@ -1077,7 +1079,7 @@
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.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})));
@@ -1096,7 +1098,7 @@
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));
@@ -1128,7 +1130,7 @@
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));
}