diff options
19 files changed, 236 insertions, 144 deletions
diff --git a/cmds/installd/Android.bp b/cmds/installd/Android.bp index 5c2211ff9a..a5462367f8 100644 --- a/cmds/installd/Android.bp +++ b/cmds/installd/Android.bp @@ -189,8 +189,8 @@ cc_binary { "liblog", "libutils", ], - static_libs: [ - "libapexd", + required: [ + "apexd" ], } diff --git a/cmds/installd/otapreopt_chroot.cpp b/cmds/installd/otapreopt_chroot.cpp index 379cf92043..c04b558e2d 100644 --- a/cmds/installd/otapreopt_chroot.cpp +++ b/cmds/installd/otapreopt_chroot.cpp @@ -20,6 +20,7 @@ #include <sys/stat.h> #include <sys/wait.h> +#include <array> #include <fstream> #include <sstream> @@ -31,10 +32,6 @@ #include <libdm/dm.h> #include <selinux/android.h> -#include <apex_file_repository.h> -#include <apex_constants.h> -#include <apexd.h> - #include "installd_constants.h" #include "otapreopt_utils.h" @@ -64,47 +61,14 @@ static void CloseDescriptor(const char* descriptor_string) { } } -static std::vector<apex::ApexFile> ActivateApexPackages() { - // The logic here is (partially) copied and adapted from - // system/apex/apexd/apexd.cpp. - // - // Only scan the APEX directory under /system, /system_ext and /vendor (within the chroot dir). - std::vector<std::string> apex_dirs{apex::kApexPackageSystemDir, apex::kApexPackageSystemExtDir, - apex::kApexPackageVendorDir}; - // Initialize ApexFileRepository used internally in ScanPackagesDirAndActivate. - // This is a quick fix to fix apex activation in otapreopt_chroot. - apex::ApexFileRepository::GetInstance().AddPreInstalledApex(apex_dirs); - for (const auto& dir : apex_dirs) { - // Cast call to void to suppress warn_unused_result. - static_cast<void>(apex::ScanPackagesDirAndActivate(dir.c_str())); - } - return apex::GetActivePackages(); -} - -static void CreateApexInfoList(const std::vector<apex::ApexFile>& apex_files) { - // Setup the apex-info-list.xml file - const std::string apex_info_file = std::string(apex::kApexRoot) + "/" + apex::kApexInfoList; - std::fstream xml(apex_info_file.c_str(), std::ios::out | std::ios::trunc); - if (!xml.is_open()) { - PLOG(ERROR) << "Failed to open " << apex_info_file; - exit(216); - } - - // we do not care about inactive apexs - std::vector<apex::ApexFile> inactive; - apex::CollectApexInfoList(xml, apex_files, inactive); - xml.flush(); - xml.close(); -} +static void ActivateApexPackages() { + std::vector<std::string> apexd_cmd{"/system/bin/apexd", "--otachroot-bootstrap"}; + std::string apexd_error_msg; -static void DeactivateApexPackages(const std::vector<apex::ApexFile>& active_packages) { - for (const apex::ApexFile& apex_file : active_packages) { - const std::string& package_path = apex_file.GetPath(); - base::Result<void> status = apex::DeactivatePackage(package_path); - if (!status.ok()) { - LOG(ERROR) << "Failed to deactivate " << package_path << ": " - << status.error(); - } + bool exec_result = Exec(apexd_cmd, &apexd_error_msg); + if (!exec_result) { + PLOG(ERROR) << "Running otapreopt failed: " << apexd_error_msg; + exit(220); } } @@ -269,8 +233,7 @@ static int otapreopt_chroot(const int argc, char **arg) { // Try to mount APEX packages in "/apex" in the chroot dir. We need at least // the ART APEX, as it is required by otapreopt to run dex2oat. - std::vector<apex::ApexFile> active_packages = ActivateApexPackages(); - CreateApexInfoList(active_packages); + ActivateApexPackages(); // Check that an ART APEX has been activated; clean up and exit // early otherwise. @@ -278,16 +241,27 @@ static int otapreopt_chroot(const int argc, char **arg) { "com.android.art", "com.android.runtime", }; - for (std::string_view apex : kRequiredApexs) { - if (std::none_of(active_packages.begin(), active_packages.end(), - [&](const apex::ApexFile& package) { - return package.GetManifest().name() == apex; - })) { - LOG(FATAL_WITHOUT_ABORT) << "No activated " << apex << " APEX package."; - DeactivateApexPackages(active_packages); - exit(217); + std::array<bool, arraysize(kRequiredApexs)> found_apexs{ false, false }; + DIR* apex_dir = opendir("/apex"); + if (apex_dir == nullptr) { + PLOG(ERROR) << "unable to open /apex"; + exit(220); + } + for (dirent* entry = readdir(apex_dir); entry != nullptr; entry = readdir(apex_dir)) { + for (int i = 0; i < found_apexs.size(); i++) { + if (kRequiredApexs[i] == std::string_view(entry->d_name)) { + found_apexs[i] = true; + break; + } } } + closedir(apex_dir); + auto it = std::find(found_apexs.cbegin(), found_apexs.cend(), false); + if (it != found_apexs.cend()) { + LOG(ERROR) << "No activated " << kRequiredApexs[std::distance(found_apexs.cbegin(), it)] + << " package!"; + exit(221); + } // Setup /linkerconfig. Doing it after the chroot means it doesn't need its own category if (selinux_android_restorecon("/linkerconfig", 0) < 0) { @@ -323,9 +297,6 @@ static int otapreopt_chroot(const int argc, char **arg) { LOG(ERROR) << "Running otapreopt failed: " << error_msg; } - // Tear down the work down by the apexd logic. (i.e. deactivate packages). - DeactivateApexPackages(active_packages); - if (!exec_result) { exit(213); } diff --git a/cmds/installd/tests/installd_dexopt_test.cpp b/cmds/installd/tests/installd_dexopt_test.cpp index fbf1e0c4b6..e27202597c 100644 --- a/cmds/installd/tests/installd_dexopt_test.cpp +++ b/cmds/installd/tests/installd_dexopt_test.cpp @@ -351,7 +351,7 @@ protected: uid = kTestAppUid; } if (class_loader_context == nullptr) { - class_loader_context = "&"; + class_loader_context = "PCL[]"; } int32_t dexopt_needed = 0; // does not matter; std::optional<std::string> out_path; // does not matter @@ -478,7 +478,7 @@ protected: bool should_binder_call_succeed, /*out */ binder::Status* binder_result) { std::optional<std::string> out_path = oat_dir ? std::make_optional<std::string>(oat_dir) : std::nullopt; - std::string class_loader_context = "&"; + std::string class_loader_context = "PCL[]"; int32_t target_sdk_version = 0; // default std::string profile_name = "primary.prof"; std::optional<std::string> dm_path_opt = dm_path ? std::make_optional<std::string>(dm_path) : std::nullopt; diff --git a/include/android/surface_control.h b/include/android/surface_control.h index eb9c5280e2..91726cd737 100644 --- a/include/android/surface_control.h +++ b/include/android/surface_control.h @@ -286,6 +286,9 @@ void ASurfaceTransaction_setZOrder(ASurfaceTransaction* transaction, * The frameworks takes ownership of the \a acquire_fence_fd passed and is responsible * for closing it. * + * Note that the buffer must be allocated with AHARDWAREBUFFER_USAGE_GPU_SAMPLED_IMAGE + * as the surface control might be composited using the GPU. + * * Available since API level 29. */ void ASurfaceTransaction_setBuffer(ASurfaceTransaction* transaction, diff --git a/libs/binder/Android.bp b/libs/binder/Android.bp index 8854ba201c..144dbd3585 100644 --- a/libs/binder/Android.bp +++ b/libs/binder/Android.bp @@ -73,6 +73,7 @@ libbinder_device_interface_sources = [ "PermissionController.cpp", "ProcessInfoService.cpp", "IpPrefix.cpp", + ":activity_manager_procstate_aidl", ] cc_library { @@ -130,8 +131,8 @@ cc_library { "Status.cpp", "TextOutput.cpp", "Utils.cpp", + ":packagemanager_aidl", ":libbinder_aidl", - ":activity_manager_procstate_aidl", ], target: { @@ -232,9 +233,6 @@ cc_library { filegroup { name: "libbinder_aidl", srcs: [ - "aidl/android/content/pm/IPackageChangeObserver.aidl", - "aidl/android/content/pm/IPackageManagerNative.aidl", - "aidl/android/content/pm/PackageChangeEvent.aidl", "aidl/android/os/IClientCallback.aidl", "aidl/android/os/IServiceCallback.aidl", "aidl/android/os/IServiceManager.aidl", @@ -243,6 +241,16 @@ filegroup { path: "aidl", } +filegroup { + name: "packagemanager_aidl", + srcs: [ + "aidl/android/content/pm/IPackageChangeObserver.aidl", + "aidl/android/content/pm/IPackageManagerNative.aidl", + "aidl/android/content/pm/PackageChangeEvent.aidl", + ], + path: "aidl", +} + aidl_interface { name: "libbinder_aidl_test_stub", unstable: true, diff --git a/libs/binder/Parcel.cpp b/libs/binder/Parcel.cpp index 6f34159699..7fedba2666 100644 --- a/libs/binder/Parcel.cpp +++ b/libs/binder/Parcel.cpp @@ -41,11 +41,12 @@ #include <binder/TextOutput.h> #include <cutils/ashmem.h> +#include <cutils/compiler.h> #include <utils/Flattenable.h> #include <utils/Log.h> -#include <utils/misc.h> -#include <utils/String8.h> #include <utils/String16.h> +#include <utils/String8.h> +#include <utils/misc.h> #include <private/binder/binder_module.h> #include "RpcState.h" @@ -590,12 +591,14 @@ status_t Parcel::writeInterfaceToken(const String16& interface) } status_t Parcel::writeInterfaceToken(const char16_t* str, size_t len) { - const IPCThreadState* threadState = IPCThreadState::self(); - writeInt32(threadState->getStrictModePolicy() | STRICT_MODE_PENALTY_GATHER); - updateWorkSourceRequestHeaderPosition(); - writeInt32(threadState->shouldPropagateWorkSource() ? - threadState->getCallingWorkSourceUid() : IPCThreadState::kUnsetWorkSource); - writeInt32(kHeader); + if (CC_LIKELY(!isForRpc())) { + const IPCThreadState* threadState = IPCThreadState::self(); + writeInt32(threadState->getStrictModePolicy() | STRICT_MODE_PENALTY_GATHER); + updateWorkSourceRequestHeaderPosition(); + writeInt32(threadState->shouldPropagateWorkSource() ? threadState->getCallingWorkSourceUid() + : IPCThreadState::kUnsetWorkSource); + writeInt32(kHeader); + } // currently the interface identification token is just its name as a string return writeString16(str, len); @@ -642,31 +645,34 @@ bool Parcel::enforceInterface(const char16_t* interface, size_t len, IPCThreadState* threadState) const { - // StrictModePolicy. - int32_t strictPolicy = readInt32(); - if (threadState == nullptr) { - threadState = IPCThreadState::self(); - } - if ((threadState->getLastTransactionBinderFlags() & - IBinder::FLAG_ONEWAY) != 0) { - // For one-way calls, the callee is running entirely - // disconnected from the caller, so disable StrictMode entirely. - // Not only does disk/network usage not impact the caller, but - // there's no way to commuicate back any violations anyway. - threadState->setStrictModePolicy(0); - } else { - threadState->setStrictModePolicy(strictPolicy); - } - // WorkSource. - updateWorkSourceRequestHeaderPosition(); - int32_t workSource = readInt32(); - threadState->setCallingWorkSourceUidWithoutPropagation(workSource); - // vendor header - int32_t header = readInt32(); - if (header != kHeader) { - ALOGE("Expecting header 0x%x but found 0x%x. Mixing copies of libbinder?", kHeader, header); - return false; + if (CC_LIKELY(!isForRpc())) { + // StrictModePolicy. + int32_t strictPolicy = readInt32(); + if (threadState == nullptr) { + threadState = IPCThreadState::self(); + } + if ((threadState->getLastTransactionBinderFlags() & IBinder::FLAG_ONEWAY) != 0) { + // For one-way calls, the callee is running entirely + // disconnected from the caller, so disable StrictMode entirely. + // Not only does disk/network usage not impact the caller, but + // there's no way to communicate back violations anyway. + threadState->setStrictModePolicy(0); + } else { + threadState->setStrictModePolicy(strictPolicy); + } + // WorkSource. + updateWorkSourceRequestHeaderPosition(); + int32_t workSource = readInt32(); + threadState->setCallingWorkSourceUidWithoutPropagation(workSource); + // vendor header + int32_t header = readInt32(); + if (header != kHeader) { + ALOGE("Expecting header 0x%x but found 0x%x. Mixing copies of libbinder?", kHeader, + header); + return false; + } } + // Interface descriptor. size_t parcel_interface_len; const char16_t* parcel_interface = readString16Inplace(&parcel_interface_len); diff --git a/libs/binder/ndk/include_platform/android/binder_manager.h b/libs/binder/ndk/include_platform/android/binder_manager.h index 5df0012bd3..55169140df 100644 --- a/libs/binder/ndk/include_platform/android/binder_manager.h +++ b/libs/binder/ndk/include_platform/android/binder_manager.h @@ -26,6 +26,9 @@ __BEGIN_DECLS * This registers the service with the default service manager under this instance name. This does * not take ownership of binder. * + * WARNING: when using this API across an APEX boundary, do not use with unstable + * AIDL services. TODO(b/139325195) + * * \param binder object to register globally with the service manager. * \param instance identifier of the service. This will be used to lookup the service. * @@ -39,6 +42,9 @@ __attribute__((warn_unused_result)) binder_exception_t AServiceManager_addServic * service is not available This also implicitly calls AIBinder_incStrong (so the caller of this * function is responsible for calling AIBinder_decStrong). * + * WARNING: when using this API across an APEX boundary, do not use with unstable + * AIDL services. TODO(b/139325195) + * * \param instance identifier of the service used to lookup the service. */ __attribute__((warn_unused_result)) AIBinder* AServiceManager_checkService(const char* instance); @@ -48,6 +54,9 @@ __attribute__((warn_unused_result)) AIBinder* AServiceManager_checkService(const * it. This also implicitly calls AIBinder_incStrong (so the caller of this function is responsible * for calling AIBinder_decStrong). * + * WARNING: when using this API across an APEX boundary, do not use with unstable + * AIDL services. TODO(b/139325195) + * * \param instance identifier of the service used to lookup the service. */ __attribute__((warn_unused_result)) AIBinder* AServiceManager_getService(const char* instance); @@ -78,6 +87,9 @@ binder_status_t AServiceManager_registerLazyService(AIBinder* binder, const char * This also implicitly calls AIBinder_incStrong (so the caller of this function is responsible * for calling AIBinder_decStrong). * + * WARNING: when using this API across an APEX boundary, do not use with unstable + * AIDL services. TODO(b/139325195) + * * \param instance identifier of the service used to lookup the service. * * \return service if registered, null if not. diff --git a/libs/binder/rust/src/binder.rs b/libs/binder/rust/src/binder.rs index 3899d47bd6..321b422185 100644 --- a/libs/binder/rust/src/binder.rs +++ b/libs/binder/rust/src/binder.rs @@ -56,6 +56,26 @@ pub trait Interface: Send { } } +/// Interface stability promise +/// +/// An interface can promise to be a stable vendor interface ([`Vintf`]), or +/// makes no stability guarantees ([`Local`]). [`Local`] is +/// currently the default stability. +pub enum Stability { + /// Default stability, visible to other modules in the same compilation + /// context (e.g. modules on system.img) + Local, + + /// A Vendor Interface Object, which promises to be stable + Vintf, +} + +impl Default for Stability { + fn default() -> Self { + Stability::Local + } +} + /// A local service that can be remotable via Binder. /// /// An object that implement this interface made be made into a Binder service @@ -94,6 +114,8 @@ pub const LAST_CALL_TRANSACTION: TransactionCode = sys::LAST_CALL_TRANSACTION; pub const FLAG_ONEWAY: TransactionFlags = sys::FLAG_ONEWAY; /// Corresponds to TF_CLEAR_BUF -- clear transaction buffers after call is made. pub const FLAG_CLEAR_BUF: TransactionFlags = sys::FLAG_CLEAR_BUF; +/// Set to the vendor flag if we are building for the VNDK, 0 otherwise +pub const FLAG_PRIVATE_LOCAL: TransactionFlags = sys::FLAG_PRIVATE_LOCAL; /// Internal interface of binder local or remote objects for making /// transactions. @@ -602,6 +624,23 @@ macro_rules! declare_binder_interface { $interface[$descriptor] { native: $native($on_transact), proxy: $proxy {}, + stability: $crate::Stability::default(), + } + } + }; + + { + $interface:path[$descriptor:expr] { + native: $native:ident($on_transact:path), + proxy: $proxy:ident, + stability: $stability:expr, + } + } => { + $crate::declare_binder_interface! { + $interface[$descriptor] { + native: $native($on_transact), + proxy: $proxy {}, + stability: $stability, } } }; @@ -616,12 +655,33 @@ macro_rules! declare_binder_interface { } => { $crate::declare_binder_interface! { $interface[$descriptor] { + native: $native($on_transact), + proxy: $proxy { + $($fname: $fty = $finit),* + }, + stability: $crate::Stability::default(), + } + } + }; + + { + $interface:path[$descriptor:expr] { + native: $native:ident($on_transact:path), + proxy: $proxy:ident { + $($fname:ident: $fty:ty = $finit:expr),* + }, + stability: $stability:expr, + } + } => { + $crate::declare_binder_interface! { + $interface[$descriptor] { @doc[concat!("A binder [`Remotable`]($crate::Remotable) that holds an [`", stringify!($interface), "`] object.")] native: $native($on_transact), @doc[concat!("A binder [`Proxy`]($crate::Proxy) that holds an [`", stringify!($interface), "`] remote interface.")] proxy: $proxy { $($fname: $fty = $finit),* }, + stability: $stability, } } }; @@ -635,6 +695,8 @@ macro_rules! declare_binder_interface { proxy: $proxy:ident { $($fname:ident: $fty:ty = $finit:expr),* }, + + stability: $stability:expr, } } => { #[doc = $proxy_doc] @@ -669,7 +731,7 @@ macro_rules! declare_binder_interface { impl $native { /// Create a new binder service. pub fn new_binder<T: $interface + Sync + Send + 'static>(inner: T) -> $crate::Strong<dyn $interface> { - let binder = $crate::Binder::new($native(Box::new(inner))); + let binder = $crate::Binder::new_with_stability($native(Box::new(inner)), $stability); $crate::Strong::new(Box::new(binder)) } } diff --git a/libs/binder/rust/src/lib.rs b/libs/binder/rust/src/lib.rs index 5bbd2a3870..30928a5cff 100644 --- a/libs/binder/rust/src/lib.rs +++ b/libs/binder/rust/src/lib.rs @@ -107,8 +107,9 @@ use binder_ndk_sys as sys; pub mod parcel; pub use crate::binder::{ - FromIBinder, IBinder, IBinderInternal, Interface, InterfaceClass, Remotable, Strong, - TransactionCode, TransactionFlags, Weak, FIRST_CALL_TRANSACTION, FLAG_CLEAR_BUF, FLAG_ONEWAY, + FromIBinder, IBinder, IBinderInternal, Interface, InterfaceClass, Remotable, + Stability, Strong, TransactionCode, TransactionFlags, Weak, + FIRST_CALL_TRANSACTION, FLAG_CLEAR_BUF, FLAG_ONEWAY, FLAG_PRIVATE_LOCAL, LAST_CALL_TRANSACTION, }; pub use error::{status_t, ExceptionCode, Result, Status, StatusCode}; diff --git a/libs/binder/rust/src/native.rs b/libs/binder/rust/src/native.rs index 185645ef4a..3b3fd08cdc 100644 --- a/libs/binder/rust/src/native.rs +++ b/libs/binder/rust/src/native.rs @@ -14,7 +14,7 @@ * limitations under the License. */ -use crate::binder::{AsNative, Interface, InterfaceClassMethods, Remotable, TransactionCode}; +use crate::binder::{AsNative, Interface, InterfaceClassMethods, Remotable, Stability, TransactionCode}; use crate::error::{status_result, status_t, Result, StatusCode}; use crate::parcel::{Parcel, Serialize}; use crate::proxy::SpIBinder; @@ -49,11 +49,19 @@ pub struct Binder<T: Remotable> { unsafe impl<T: Remotable> Send for Binder<T> {} impl<T: Remotable> Binder<T> { - /// Create a new Binder remotable object. + /// Create a new Binder remotable object with default stability /// /// This moves the `rust_object` into an owned [`Box`] and Binder will /// manage its lifetime. pub fn new(rust_object: T) -> Binder<T> { + Self::new_with_stability(rust_object, Stability::default()) + } + + /// Create a new Binder remotable object with the given stability + /// + /// This moves the `rust_object` into an owned [`Box`] and Binder will + /// manage its lifetime. + pub fn new_with_stability(rust_object: T, stability: Stability) -> Binder<T> { let class = T::get_class(); let rust_object = Box::into_raw(Box::new(rust_object)); let ibinder = unsafe { @@ -65,10 +73,12 @@ impl<T: Remotable> Binder<T> { // ends. sys::AIBinder_new(class.into(), rust_object as *mut c_void) }; - Binder { + let mut binder = Binder { ibinder, rust_object, - } + }; + binder.mark_stability(stability); + binder } /// Set the extension of a binder interface. This allows a downstream @@ -161,6 +171,42 @@ impl<T: Remotable> Binder<T> { pub fn get_descriptor() -> &'static str { T::get_descriptor() } + + /// Mark this binder object with the given stability guarantee + fn mark_stability(&mut self, stability: Stability) { + match stability { + Stability::Local => self.mark_local_stability(), + Stability::Vintf => { + unsafe { + // Safety: Self always contains a valid `AIBinder` pointer, so + // we can always call this C API safely. + sys::AIBinder_markVintfStability(self.as_native_mut()); + } + } + } + } + + /// Mark this binder object with local stability, which is vendor if we are + /// building for the VNDK and system otherwise. + #[cfg(vendor_ndk)] + fn mark_local_stability(&mut self) { + unsafe { + // Safety: Self always contains a valid `AIBinder` pointer, so + // we can always call this C API safely. + sys::AIBinder_markVendorStability(self.as_native_mut()); + } + } + + /// Mark this binder object with local stability, which is vendor if we are + /// building for the VNDK and system otherwise. + #[cfg(not(vendor_ndk))] + fn mark_local_stability(&mut self) { + unsafe { + // Safety: Self always contains a valid `AIBinder` pointer, so + // we can always call this C API safely. + sys::AIBinder_markSystemStability(self.as_native_mut()); + } + } } impl<T: Remotable> Interface for Binder<T> { diff --git a/libs/binder/rust/sys/BinderBindings.hpp b/libs/binder/rust/sys/BinderBindings.hpp index ef142b5cf8..65fa2ca905 100644 --- a/libs/binder/rust/sys/BinderBindings.hpp +++ b/libs/binder/rust/sys/BinderBindings.hpp @@ -21,6 +21,7 @@ #include <android/binder_parcel_platform.h> #include <android/binder_process.h> #include <android/binder_shell.h> +#include <android/binder_stability.h> #include <android/binder_status.h> namespace android { @@ -80,6 +81,7 @@ enum { enum { FLAG_ONEWAY = FLAG_ONEWAY, FLAG_CLEAR_BUF = FLAG_CLEAR_BUF, + FLAG_PRIVATE_LOCAL = FLAG_PRIVATE_LOCAL, }; } // namespace consts diff --git a/libs/renderengine/skia/AutoBackendTexture.cpp b/libs/renderengine/skia/AutoBackendTexture.cpp index c535597aea..ae3d88a9fb 100644 --- a/libs/renderengine/skia/AutoBackendTexture.cpp +++ b/libs/renderengine/skia/AutoBackendTexture.cpp @@ -28,19 +28,19 @@ namespace android { namespace renderengine { namespace skia { -AutoBackendTexture::AutoBackendTexture(GrDirectContext* context, AHardwareBuffer* buffer, - bool isRender) { +AutoBackendTexture::AutoBackendTexture(GrDirectContext* context, AHardwareBuffer* buffer) { ATRACE_CALL(); AHardwareBuffer_Desc desc; AHardwareBuffer_describe(buffer, &desc); - bool createProtectedImage = 0 != (desc.usage & AHARDWAREBUFFER_USAGE_PROTECTED_CONTENT); + const bool createProtectedImage = 0 != (desc.usage & AHARDWAREBUFFER_USAGE_PROTECTED_CONTENT); + const bool isRenderable = 0 != (desc.usage & AHARDWAREBUFFER_USAGE_GPU_FRAMEBUFFER); GrBackendFormat backendFormat = GrAHardwareBufferUtils::GetBackendFormat(context, buffer, desc.format, false); mBackendTexture = GrAHardwareBufferUtils::MakeBackendTexture(context, buffer, desc.width, desc.height, &mDeleteProc, &mUpdateProc, &mImageCtx, createProtectedImage, backendFormat, - isRender); + isRenderable); mColorType = GrAHardwareBufferUtils::GetSkColorTypeFromBufferFormat(desc.format); } diff --git a/libs/renderengine/skia/AutoBackendTexture.h b/libs/renderengine/skia/AutoBackendTexture.h index bb758780e1..a6f73db3dc 100644 --- a/libs/renderengine/skia/AutoBackendTexture.h +++ b/libs/renderengine/skia/AutoBackendTexture.h @@ -69,7 +69,7 @@ public: }; // Creates a GrBackendTexture whose contents come from the provided buffer. - AutoBackendTexture(GrDirectContext* context, AHardwareBuffer* buffer, bool isRender); + AutoBackendTexture(GrDirectContext* context, AHardwareBuffer* buffer); void ref() { mUsageCount++; } diff --git a/libs/renderengine/skia/SkiaGLRenderEngine.cpp b/libs/renderengine/skia/SkiaGLRenderEngine.cpp index c5ee15df29..c7001b9851 100644 --- a/libs/renderengine/skia/SkiaGLRenderEngine.cpp +++ b/libs/renderengine/skia/SkiaGLRenderEngine.cpp @@ -499,7 +499,7 @@ void SkiaGLRenderEngine::cacheExternalTextureBuffer(const sp<GraphicBuffer>& buf std::shared_ptr<AutoBackendTexture::LocalRef> imageTextureRef = std::make_shared<AutoBackendTexture::LocalRef>(); imageTextureRef->setTexture( - new AutoBackendTexture(grContext.get(), buffer->toAHardwareBuffer(), false)); + new AutoBackendTexture(grContext.get(), buffer->toAHardwareBuffer())); cache.insert({buffer->getId(), imageTextureRef}); } // restore the original state of the protected context if necessary @@ -653,7 +653,7 @@ status_t SkiaGLRenderEngine::drawLayers(const DisplaySettings& display, ATRACE_NAME("Cache miss"); surfaceTextureRef = std::make_shared<AutoBackendTexture::LocalRef>(); surfaceTextureRef->setTexture( - new AutoBackendTexture(grContext.get(), buffer->toAHardwareBuffer(), true)); + new AutoBackendTexture(grContext.get(), buffer->toAHardwareBuffer())); if (useFramebufferCache) { ALOGD("Adding to cache"); cache.insert({buffer->getId(), surfaceTextureRef}); @@ -860,9 +860,8 @@ status_t SkiaGLRenderEngine::drawLayers(const DisplaySettings& display, imageTextureRef = iter->second; } else { imageTextureRef = std::make_shared<AutoBackendTexture::LocalRef>(); - imageTextureRef->setTexture(new AutoBackendTexture(grContext.get(), - item.buffer->toAHardwareBuffer(), - false)); + imageTextureRef->setTexture( + new AutoBackendTexture(grContext.get(), item.buffer->toAHardwareBuffer())); cache.insert({item.buffer->getId(), imageTextureRef}); } diff --git a/services/surfaceflinger/CompositionEngine/include/compositionengine/impl/planner/CachedSet.h b/services/surfaceflinger/CompositionEngine/include/compositionengine/impl/planner/CachedSet.h index b0e42b7692..a56f28aec0 100644 --- a/services/surfaceflinger/CompositionEngine/include/compositionengine/impl/planner/CachedSet.h +++ b/services/surfaceflinger/CompositionEngine/include/compositionengine/impl/planner/CachedSet.h @@ -70,7 +70,7 @@ public: size_t getCreationCost() const; size_t getDisplayCost() const; - bool hasBufferUpdate(std::vector<const LayerState*>::const_iterator layers) const; + bool hasBufferUpdate() const; bool hasReadyBuffer() const; // Decomposes this CachedSet into a vector of its layers as individual CachedSets diff --git a/services/surfaceflinger/CompositionEngine/src/planner/CachedSet.cpp b/services/surfaceflinger/CompositionEngine/src/planner/CachedSet.cpp index 137697b59d..4d3036a686 100644 --- a/services/surfaceflinger/CompositionEngine/src/planner/CachedSet.cpp +++ b/services/surfaceflinger/CompositionEngine/src/planner/CachedSet.cpp @@ -116,12 +116,11 @@ size_t CachedSet::getDisplayCost() const { return static_cast<size_t>(mBounds.width() * mBounds.height()); } -bool CachedSet::hasBufferUpdate(std::vector<const LayerState*>::const_iterator layers) const { +bool CachedSet::hasBufferUpdate() const { for (const Layer& layer : mLayers) { if (layer.getFramesSinceBufferUpdate() == 0) { return true; } - ++layers; } return false; } diff --git a/services/surfaceflinger/CompositionEngine/src/planner/Flattener.cpp b/services/surfaceflinger/CompositionEngine/src/planner/Flattener.cpp index 4dad4e63c4..60ebbb2c65 100644 --- a/services/surfaceflinger/CompositionEngine/src/planner/Flattener.cpp +++ b/services/surfaceflinger/CompositionEngine/src/planner/Flattener.cpp @@ -196,7 +196,7 @@ bool Flattener::mergeWithCachedSets(const std::vector<const LayerState*>& layers if (mNewCachedSet && mNewCachedSet->getFingerprint() == (*incomingLayerIter)->getHash(LayerStateField::Buffer)) { - if (mNewCachedSet->hasBufferUpdate(incomingLayerIter)) { + if (mNewCachedSet->hasBufferUpdate()) { ALOGV("[%s] Dropping new cached set", __func__); ++mInvalidatedCachedSetAges[0]; mNewCachedSet = std::nullopt; @@ -230,7 +230,7 @@ bool Flattener::mergeWithCachedSets(const std::vector<const LayerState*>& layers } } - if (!currentLayerIter->hasBufferUpdate(incomingLayerIter)) { + if (!currentLayerIter->hasBufferUpdate()) { currentLayerIter->incrementAge(); merged.emplace_back(*currentLayerIter); diff --git a/services/surfaceflinger/CompositionEngine/tests/planner/CachedSetTest.cpp b/services/surfaceflinger/CompositionEngine/tests/planner/CachedSetTest.cpp index 377f817ae8..7842efbe00 100644 --- a/services/surfaceflinger/CompositionEngine/tests/planner/CachedSetTest.cpp +++ b/services/surfaceflinger/CompositionEngine/tests/planner/CachedSetTest.cpp @@ -201,13 +201,7 @@ TEST_F(CachedSetTest, hasBufferUpdate_NoUpdate) { cachedSet.addLayer(layer2.getState(), kStartTime + 10ms); cachedSet.addLayer(layer3.getState(), kStartTime + 20ms); - std::vector<const LayerState*> incomingLayers = { - layer1.getState(), - layer2.getState(), - layer3.getState(), - }; - - EXPECT_FALSE(cachedSet.hasBufferUpdate(incomingLayers.begin())); + EXPECT_FALSE(cachedSet.hasBufferUpdate()); } TEST_F(CachedSetTest, hasBufferUpdate_BufferUpdate) { @@ -221,13 +215,7 @@ TEST_F(CachedSetTest, hasBufferUpdate_BufferUpdate) { mTestLayers[1]->layerState->resetFramesSinceBufferUpdate(); - std::vector<const LayerState*> incomingLayers = { - layer1.getState(), - layer2.getState(), - layer3.getState(), - }; - - EXPECT_TRUE(cachedSet.hasBufferUpdate(incomingLayers.begin())); + EXPECT_TRUE(cachedSet.hasBufferUpdate()); } TEST_F(CachedSetTest, append) { diff --git a/services/surfaceflinger/SurfaceFlinger.cpp b/services/surfaceflinger/SurfaceFlinger.cpp index 525b04364b..a387587d7f 100644 --- a/services/surfaceflinger/SurfaceFlinger.cpp +++ b/services/surfaceflinger/SurfaceFlinger.cpp @@ -3450,6 +3450,7 @@ bool SurfaceFlinger::transactionIsReadyToBeApplied( const bool acquireFenceChanged = (s.what & layer_state_t::eAcquireFenceChanged); if (acquireFenceChanged && s.acquireFence && s.acquireFence->getStatus() == Fence::Status::Unsignaled) { + ATRACE_NAME("fence unsignaled"); ready = false; } @@ -5926,8 +5927,11 @@ status_t SurfaceFlinger::captureScreenCommon(RenderAreaFuture renderAreaFuture, const status_t bufferStatus = buffer->initCheck(); LOG_ALWAYS_FATAL_IF(bufferStatus != OK, "captureScreenCommon: Buffer failed to allocate: %d", bufferStatus); - return captureScreenCommon(std::move(renderAreaFuture), traverseLayers, buffer, - false /* regionSampling */, grayscale, captureListener); + getRenderEngine().cacheExternalTextureBuffer(buffer); + status_t result = captureScreenCommon(std::move(renderAreaFuture), traverseLayers, buffer, + false /* regionSampling */, grayscale, captureListener); + getRenderEngine().unbindExternalTextureBuffer(buffer->getId()); + return result; } status_t SurfaceFlinger::captureScreenCommon(RenderAreaFuture renderAreaFuture, @@ -5967,15 +5971,6 @@ status_t SurfaceFlinger::captureScreenCommon(RenderAreaFuture renderAreaFuture, regionSampling, grayscale, captureResults); }); - // TODO(b/180767535): Remove this once we optimize buffer lifecycle for RenderEngine - // Only do this when we're not doing region sampling, to allow the region sampling thread to - // manage buffer lifecycle itself. - if (!regionSampling && - getRenderEngine().getRenderEngineType() == - renderengine::RenderEngine::RenderEngineType::SKIA_GL_THREADED) { - getRenderEngine().unbindExternalTextureBuffer(buffer->getId()); - } - captureResults.result = result; captureListener->onScreenCaptureCompleted(captureResults); })); |