diff options
40 files changed, 1064 insertions, 281 deletions
diff --git a/cmds/dumpstate/dumpstate.cpp b/cmds/dumpstate/dumpstate.cpp index 0bbd4a86ba..481c28ef6b 100644 --- a/cmds/dumpstate/dumpstate.cpp +++ b/cmds/dumpstate/dumpstate.cpp @@ -199,6 +199,7 @@ static const std::string TOMBSTONE_DIR = "/data/tombstones/"; static const std::string TOMBSTONE_FILE_PREFIX = "tombstone_"; static const std::string ANR_DIR = "/data/anr/"; static const std::string ANR_FILE_PREFIX = "anr_"; +static const std::string ANR_TRACE_FILE_PREFIX = "trace_"; static const std::string SHUTDOWN_CHECKPOINTS_DIR = "/data/system/shutdown-checkpoints/"; static const std::string SHUTDOWN_CHECKPOINTS_FILE_PREFIX = "checkpoints-"; @@ -1186,6 +1187,10 @@ static void AddAnrTraceDir(const std::string& anr_traces_dir) { } else { printf("*** NO ANRs to dump in %s\n\n", ANR_DIR.c_str()); } + + // Add Java anr traces (such as generated by the Finalizer Watchdog). + AddDumps(ds.anr_trace_data_.begin(), ds.anr_trace_data_.end(), "JAVA ANR TRACES", + true /* add_to_zip */); } static void AddAnrTraceFiles() { @@ -1904,6 +1909,7 @@ Dumpstate::RunStatus Dumpstate::DumpstateDefaultAfterCritical() { if (!PropertiesHelper::IsDryRun()) { ds.tombstone_data_ = GetDumpFds(TOMBSTONE_DIR, TOMBSTONE_FILE_PREFIX); ds.anr_data_ = GetDumpFds(ANR_DIR, ANR_FILE_PREFIX); + ds.anr_trace_data_ = GetDumpFds(ANR_DIR, ANR_TRACE_FILE_PREFIX); ds.shutdown_checkpoints_ = GetDumpFds( SHUTDOWN_CHECKPOINTS_DIR, SHUTDOWN_CHECKPOINTS_FILE_PREFIX); } @@ -3064,6 +3070,7 @@ void Dumpstate::Cancel() { } tombstone_data_.clear(); anr_data_.clear(); + anr_trace_data_.clear(); shutdown_checkpoints_.clear(); // Instead of shutdown the pool, we delete temporary files directly since @@ -3364,6 +3371,7 @@ Dumpstate::RunStatus Dumpstate::RunInternal(int32_t calling_uid, tombstone_data_.clear(); anr_data_.clear(); + anr_trace_data_.clear(); shutdown_checkpoints_.clear(); return (consent_callback_ != nullptr && diff --git a/cmds/dumpstate/dumpstate.h b/cmds/dumpstate/dumpstate.h index 20b28656ce..46d949e303 100644 --- a/cmds/dumpstate/dumpstate.h +++ b/cmds/dumpstate/dumpstate.h @@ -526,6 +526,9 @@ class Dumpstate { // List of open ANR dump files. std::vector<DumpData> anr_data_; + // List of open Java traces files in the anr directory. + std::vector<DumpData> anr_trace_data_; + // List of open shutdown checkpoint files. std::vector<DumpData> shutdown_checkpoints_; diff --git a/cmds/flatland/Android.bp b/cmds/flatland/Android.bp new file mode 100644 index 0000000000..39a0d752a5 --- /dev/null +++ b/cmds/flatland/Android.bp @@ -0,0 +1,54 @@ +/* + * Copyright (C) 2024 The Android Open Source Project + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at: + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package { + // See: http://go/android-license-faq + default_applicable_licenses: [ + "frameworks_native_license", + ], +} + +cc_benchmark { + name: "flatland", + auto_gen_config: false, + srcs: [ + "Composers.cpp", + "GLHelper.cpp", + "Renderers.cpp", + "Main.cpp", + ], + cflags: [ + "-Wall", + "-Werror", + ], + compile_multilib: "both", + multilib: { + lib32: { + stem: "flatland", + }, + lib64: { + stem: "flatland64", + }, + }, + shared_libs: [ + "libEGL", + "libGLESv2", + "libcutils", + "libgui", + "libui", + "libutils", + ], +} diff --git a/cmds/flatland/Android.mk b/cmds/flatland/Android.mk deleted file mode 100644 index 754a99caf6..0000000000 --- a/cmds/flatland/Android.mk +++ /dev/null @@ -1,32 +0,0 @@ -local_target_dir := $(TARGET_OUT_DATA)/local/tmp -LOCAL_PATH:= $(call my-dir) -include $(CLEAR_VARS) - -LOCAL_SRC_FILES:= \ - Composers.cpp \ - GLHelper.cpp \ - Renderers.cpp \ - Main.cpp \ - -LOCAL_CFLAGS := -Wall -Werror - -LOCAL_MODULE:= flatland -LOCAL_LICENSE_KINDS:= SPDX-license-identifier-Apache-2.0 -LOCAL_LICENSE_CONDITIONS:= notice -LOCAL_NOTICE_FILE:= $(LOCAL_PATH)/../../NOTICE - -LOCAL_MODULE_TAGS := tests - -LOCAL_MODULE_PATH := $(local_target_dir) -LOCAL_MULTILIB := both -LOCAL_MODULE_STEM_32 := flatland -LOCAL_MODULE_STEM_64 := flatland64 -LOCAL_SHARED_LIBRARIES := \ - libEGL \ - libGLESv2 \ - libcutils \ - libgui \ - libui \ - libutils \ - -include $(BUILD_EXECUTABLE) diff --git a/cmds/installd/InstalldNativeService.cpp b/cmds/installd/InstalldNativeService.cpp index c8ab8c07e6..7478f291dd 100644 --- a/cmds/installd/InstalldNativeService.cpp +++ b/cmds/installd/InstalldNativeService.cpp @@ -233,12 +233,12 @@ binder::Status checkArgumentFileName(const std::string& path) { return ok(); } -binder::Status checkUidInAppRange(int32_t appUid) { - if (FIRST_APPLICATION_UID <= appUid && appUid <= LAST_APPLICATION_UID) { +binder::Status checkArgumentAppId(int32_t appId) { + if (FIRST_APPLICATION_UID <= appId && appId <= LAST_APPLICATION_UID) { return ok(); } return exception(binder::Status::EX_ILLEGAL_ARGUMENT, - StringPrintf("UID %d is outside of the range", appUid)); + StringPrintf("appId %d is outside of the range", appId)); } #define ENFORCE_UID(uid) { \ @@ -301,12 +301,12 @@ binder::Status checkUidInAppRange(int32_t appUid) { } \ } -#define CHECK_ARGUMENT_UID_IN_APP_RANGE(uid) \ - { \ - binder::Status status = checkUidInAppRange((uid)); \ - if (!status.isOk()) { \ - return status; \ - } \ +#define CHECK_ARGUMENT_APP_ID(appId) \ + { \ + binder::Status status = checkArgumentAppId((appId)); \ + if (!status.isOk()) { \ + return status; \ + } \ } #ifdef GRANULAR_LOCKS @@ -410,7 +410,7 @@ using PackageLockGuard = std::lock_guard<PackageLock>; } // namespace binder::Status InstalldNativeService::FsveritySetupAuthToken::authenticate( - const ParcelFileDescriptor& authFd, int32_t appUid, int32_t userId) { + const ParcelFileDescriptor& authFd, int32_t uid) { int open_flags = fcntl(authFd.get(), F_GETFL); if (open_flags < 0) { return exception(binder::Status::EX_SERVICE_SPECIFIC, "fcntl failed"); @@ -425,9 +425,8 @@ binder::Status InstalldNativeService::FsveritySetupAuthToken::authenticate( return exception(binder::Status::EX_SECURITY, "Not a regular file"); } // Don't accept a file owned by a different app. - uid_t uid = multiuser_get_uid(userId, appUid); - if (this->mStatFromAuthFd.st_uid != uid) { - return exception(binder::Status::EX_SERVICE_SPECIFIC, "File not owned by appUid"); + if (this->mStatFromAuthFd.st_uid != (uid_t)uid) { + return exception(binder::Status::EX_SERVICE_SPECIFIC, "File not owned by uid"); } return ok(); } @@ -3974,7 +3973,7 @@ binder::Status InstalldNativeService::getOdexVisibility( // attacker-in-the-middle cannot enable fs-verity on arbitrary app files. If the FD is not writable, // return null. // -// appUid and userId are passed for additional ownership check, such that one app can not be +// app process uid is passed for additional ownership check, such that one app can not be // authenticated for another app's file. These parameters are assumed trusted for this purpose of // consistency check. // @@ -3982,13 +3981,13 @@ binder::Status InstalldNativeService::getOdexVisibility( // Since enabling fs-verity to a file requires no outstanding writable FD, passing the authFd to the // server allows the server to hold the only reference (as long as the client app doesn't). binder::Status InstalldNativeService::createFsveritySetupAuthToken( - const ParcelFileDescriptor& authFd, int32_t appUid, int32_t userId, + const ParcelFileDescriptor& authFd, int32_t uid, sp<IFsveritySetupAuthToken>* _aidl_return) { - CHECK_ARGUMENT_UID_IN_APP_RANGE(appUid); - ENFORCE_VALID_USER(userId); + CHECK_ARGUMENT_APP_ID(multiuser_get_app_id(uid)); + ENFORCE_VALID_USER(multiuser_get_user_id(uid)); auto token = sp<FsveritySetupAuthToken>::make(); - binder::Status status = token->authenticate(authFd, appUid, userId); + binder::Status status = token->authenticate(authFd, uid); if (!status.isOk()) { return status; } diff --git a/cmds/installd/InstalldNativeService.h b/cmds/installd/InstalldNativeService.h index 1b56144061..b13d6d7580 100644 --- a/cmds/installd/InstalldNativeService.h +++ b/cmds/installd/InstalldNativeService.h @@ -44,8 +44,7 @@ public: public: FsveritySetupAuthToken() : mStatFromAuthFd() {} - binder::Status authenticate(const android::os::ParcelFileDescriptor& authFd, int32_t appUid, - int32_t userId); + binder::Status authenticate(const android::os::ParcelFileDescriptor& authFd, int32_t uid); bool isSameStat(const struct stat& st) const; private: @@ -210,7 +209,7 @@ public: int32_t* _aidl_return); binder::Status createFsveritySetupAuthToken(const android::os::ParcelFileDescriptor& authFd, - int32_t appUid, int32_t userId, + int32_t uid, android::sp<IFsveritySetupAuthToken>* _aidl_return); binder::Status enableFsverity(const android::sp<IFsveritySetupAuthToken>& authToken, const std::string& filePath, const std::string& packageName, diff --git a/cmds/installd/binder/android/os/IInstalld.aidl b/cmds/installd/binder/android/os/IInstalld.aidl index f5a770977c..8a2f113bde 100644 --- a/cmds/installd/binder/android/os/IInstalld.aidl +++ b/cmds/installd/binder/android/os/IInstalld.aidl @@ -143,8 +143,7 @@ interface IInstalld { // // We don't necessarily need a method here, so it's left blank intentionally. } - IFsveritySetupAuthToken createFsveritySetupAuthToken(in ParcelFileDescriptor authFd, int appUid, - int userId); + IFsveritySetupAuthToken createFsveritySetupAuthToken(in ParcelFileDescriptor authFd, int uid); int enableFsverity(in IFsveritySetupAuthToken authToken, @utf8InCpp String filePath, @utf8InCpp String packageName); diff --git a/cmds/installd/tests/installd_service_test.cpp b/cmds/installd/tests/installd_service_test.cpp index 4bc92afa18..f2b578a8d2 100644 --- a/cmds/installd/tests/installd_service_test.cpp +++ b/cmds/installd/tests/installd_service_test.cpp @@ -548,8 +548,7 @@ protected: unique_fd ufd(open(path.c_str(), open_mode)); EXPECT_GE(ufd.get(), 0) << "open failed: " << strerror(errno); ParcelFileDescriptor rfd(std::move(ufd)); - return service->createFsveritySetupAuthToken(std::move(rfd), kTestAppId, kTestUserId, - _aidl_return); + return service->createFsveritySetupAuthToken(std::move(rfd), kTestAppId, _aidl_return); } }; diff --git a/include/input/AccelerationCurve.h b/include/input/AccelerationCurve.h new file mode 100644 index 0000000000..0cf648a2f7 --- /dev/null +++ b/include/input/AccelerationCurve.h @@ -0,0 +1,49 @@ +/* + * Copyright 2024 The Android Open Source Project + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +#pragma once + +#include <cstdint> +#include <vector> + +namespace android { + +/** + * Describes a section of an acceleration curve as a function which outputs a scaling factor (gain) + * for the pointer movement, given the speed of the mouse or finger (in mm/s): + * + * gain(input_speed_mm_per_s) = baseGain + reciprocal / input_speed_mm_per_s + */ +struct AccelerationCurveSegment { + /** + * The maximum pointer speed at which this segment should apply, in mm/s. The last segment in a + * curve should always set this to infinity. + */ + double maxPointerSpeedMmPerS; + /** The gain for this segment before the reciprocal is taken into account. */ + double baseGain; + /** The reciprocal part of the formula, which should be divided by the input speed. */ + double reciprocal; +}; + +/** + * Creates an acceleration curve for the given pointer sensitivity value. The sensitivity value + * should be between -7 (for the lowest sensitivity) and 7, inclusive. + */ +std::vector<AccelerationCurveSegment> createAccelerationCurveForPointerSensitivity( + int32_t sensitivity); + +} // namespace android diff --git a/include/input/VelocityControl.h b/include/input/VelocityControl.h index b78f63e1ae..7c58c87f8b 100644 --- a/include/input/VelocityControl.h +++ b/include/input/VelocityControl.h @@ -16,7 +16,10 @@ #pragma once +#include <vector> + #include <android-base/stringprintf.h> +#include <input/AccelerationCurve.h> #include <input/Input.h> #include <input/VelocityTracker.h> #include <utils/Timers.h> @@ -86,12 +89,7 @@ struct VelocityControlParameters { class VelocityControl { public: VelocityControl(); - - /* Gets the various parameters. */ - const VelocityControlParameters& getParameters() const; - - /* Sets the various parameters. */ - void setParameters(const VelocityControlParameters& parameters); + virtual ~VelocityControl() {} /* Resets the current movement counters to zero. * This has the effect of nullifying any acceleration. */ @@ -101,16 +99,55 @@ public: * scaled / accelerated delta based on the current velocity. */ void move(nsecs_t eventTime, float* deltaX, float* deltaY); -private: +protected: + virtual void scaleDeltas(float* deltaX, float* deltaY) = 0; + // If no movements are received within this amount of time, // we assume the movement has stopped and reset the movement counters. static const nsecs_t STOP_TIME = 500 * 1000000; // 500 ms - VelocityControlParameters mParameters; - nsecs_t mLastMovementTime; float mRawPositionX, mRawPositionY; VelocityTracker mVelocityTracker; }; +/** + * Velocity control using a simple acceleration curve where the acceleration factor increases + * linearly with movement speed, subject to minimum and maximum values. + */ +class SimpleVelocityControl : public VelocityControl { +public: + /** Gets the various parameters. */ + const VelocityControlParameters& getParameters() const; + + /** Sets the various parameters. */ + void setParameters(const VelocityControlParameters& parameters); + +protected: + virtual void scaleDeltas(float* deltaX, float* deltaY) override; + +private: + VelocityControlParameters mParameters; +}; + +/** Velocity control using a curve made up of multiple reciprocal segments. */ +class CurvedVelocityControl : public VelocityControl { +public: + CurvedVelocityControl(); + + /** Sets the curve to be used for acceleration. */ + void setCurve(const std::vector<AccelerationCurveSegment>& curve); + + void setAccelerationEnabled(bool enabled); + +protected: + virtual void scaleDeltas(float* deltaX, float* deltaY) override; + +private: + const AccelerationCurveSegment& segmentForSpeed(float speedMmPerS); + + bool mAccelerationEnabled = true; + std::vector<AccelerationCurveSegment> mCurveSegments; +}; + } // namespace android diff --git a/libs/binder/rust/src/lib.rs b/libs/binder/rust/src/lib.rs index 7f9348d913..16049f28c5 100644 --- a/libs/binder/rust/src/lib.rs +++ b/libs/binder/rust/src/lib.rs @@ -136,8 +136,8 @@ pub mod binder_impl { pub use crate::native::Binder; pub use crate::parcel::{ BorrowedParcel, Deserialize, DeserializeArray, DeserializeOption, Parcel, - ParcelableMetadata, Serialize, SerializeArray, SerializeOption, NON_NULL_PARCELABLE_FLAG, - NULL_PARCELABLE_FLAG, + ParcelableMetadata, Serialize, SerializeArray, SerializeOption, UnstructuredParcelable, + NON_NULL_PARCELABLE_FLAG, NULL_PARCELABLE_FLAG, }; pub use crate::proxy::{AssociateClass, Proxy}; } diff --git a/libs/binder/rust/src/parcel.rs b/libs/binder/rust/src/parcel.rs index f9f135d572..3bfc425ee3 100644 --- a/libs/binder/rust/src/parcel.rs +++ b/libs/binder/rust/src/parcel.rs @@ -34,7 +34,7 @@ mod parcelable_holder; pub use self::file_descriptor::ParcelFileDescriptor; pub use self::parcelable::{ Deserialize, DeserializeArray, DeserializeOption, Parcelable, Serialize, SerializeArray, - SerializeOption, NON_NULL_PARCELABLE_FLAG, NULL_PARCELABLE_FLAG, + SerializeOption, UnstructuredParcelable, NON_NULL_PARCELABLE_FLAG, NULL_PARCELABLE_FLAG, }; pub use self::parcelable_holder::{ParcelableHolder, ParcelableMetadata}; diff --git a/libs/binder/rust/src/parcel/parcelable.rs b/libs/binder/rust/src/parcel/parcelable.rs index 9008a3cc0e..33dfe19fe9 100644 --- a/libs/binder/rust/src/parcel/parcelable.rs +++ b/libs/binder/rust/src/parcel/parcelable.rs @@ -27,7 +27,7 @@ use std::os::raw::c_char; use std::ptr; use std::slice; -/// Super-trait for Binder parcelables. +/// Super-trait for structured Binder parcelables, i.e. those generated from AIDL. /// /// This trait is equivalent `android::Parcelable` in C++, /// and defines a common interface that all parcelables need @@ -50,6 +50,35 @@ pub trait Parcelable { fn read_from_parcel(&mut self, parcel: &BorrowedParcel<'_>) -> Result<()>; } +/// Super-trait for unstructured Binder parcelables, i.e. those implemented manually. +/// +/// These differ from structured parcelables in that they may not have a reasonable default value +/// and so aren't required to implement `Default`. +pub trait UnstructuredParcelable: Sized { + /// Internal serialization function for parcelables. + /// + /// This method is mainly for internal use. `Serialize::serialize` and its variants are + /// generally preferred over calling this function, since the former also prepend a header. + fn write_to_parcel(&self, parcel: &mut BorrowedParcel<'_>) -> Result<()>; + + /// Internal deserialization function for parcelables. + /// + /// This method is mainly for internal use. `Deserialize::deserialize` and its variants are + /// generally preferred over calling this function, since the former also parse the additional + /// header. + fn from_parcel(parcel: &BorrowedParcel<'_>) -> Result<Self>; + + /// Internal deserialization function for parcelables. + /// + /// This method is mainly for internal use. `Deserialize::deserialize_from` and its variants are + /// generally preferred over calling this function, since the former also parse the additional + /// header. + fn read_from_parcel(&mut self, parcel: &BorrowedParcel<'_>) -> Result<()> { + *self = Self::from_parcel(parcel)?; + Ok(()) + } +} + /// A struct whose instances can be written to a [`crate::parcel::Parcel`]. // Might be able to hook this up as a serde backend in the future? pub trait Serialize { @@ -1002,6 +1031,125 @@ macro_rules! impl_deserialize_for_parcelable { }; } +/// Implements `Serialize` trait and friends for an unstructured parcelable. +/// +/// The target type must implement the `UnstructuredParcelable` trait. +#[macro_export] +macro_rules! impl_serialize_for_unstructured_parcelable { + ($parcelable:ident) => { + $crate::impl_serialize_for_unstructured_parcelable!($parcelable < >); + }; + ($parcelable:ident < $( $param:ident ),* , >) => { + $crate::impl_serialize_for_unstructured_parcelable!($parcelable < $($param),* >); + }; + ($parcelable:ident < $( $param:ident ),* > ) => { + impl < $($param),* > $crate::binder_impl::Serialize for $parcelable < $($param),* > { + fn serialize( + &self, + parcel: &mut $crate::binder_impl::BorrowedParcel<'_>, + ) -> std::result::Result<(), $crate::StatusCode> { + <Self as $crate::binder_impl::SerializeOption>::serialize_option(Some(self), parcel) + } + } + + impl < $($param),* > $crate::binder_impl::SerializeArray for $parcelable < $($param),* > {} + + impl < $($param),* > $crate::binder_impl::SerializeOption for $parcelable < $($param),* > { + fn serialize_option( + this: Option<&Self>, + parcel: &mut $crate::binder_impl::BorrowedParcel<'_>, + ) -> std::result::Result<(), $crate::StatusCode> { + if let Some(this) = this { + use $crate::binder_impl::UnstructuredParcelable; + parcel.write(&$crate::binder_impl::NON_NULL_PARCELABLE_FLAG)?; + this.write_to_parcel(parcel) + } else { + parcel.write(&$crate::binder_impl::NULL_PARCELABLE_FLAG) + } + } + } + }; +} + +/// Implement `Deserialize` trait and friends for an unstructured parcelable +/// +/// The target type must implement the `UnstructuredParcelable` trait. +#[macro_export] +macro_rules! impl_deserialize_for_unstructured_parcelable { + ($parcelable:ident) => { + $crate::impl_deserialize_for_unstructured_parcelable!($parcelable < >); + }; + ($parcelable:ident < $( $param:ident ),* , >) => { + $crate::impl_deserialize_for_unstructured_parcelable!($parcelable < $($param),* >); + }; + ($parcelable:ident < $( $param:ident ),* > ) => { + impl < $($param: Default),* > $crate::binder_impl::Deserialize for $parcelable < $($param),* > { + type UninitType = Option<Self>; + fn uninit() -> Self::UninitType { None } + fn from_init(value: Self) -> Self::UninitType { Some(value) } + fn deserialize( + parcel: &$crate::binder_impl::BorrowedParcel<'_>, + ) -> std::result::Result<Self, $crate::StatusCode> { + $crate::binder_impl::DeserializeOption::deserialize_option(parcel) + .transpose() + .unwrap_or(Err($crate::StatusCode::UNEXPECTED_NULL)) + } + fn deserialize_from( + &mut self, + parcel: &$crate::binder_impl::BorrowedParcel<'_>, + ) -> std::result::Result<(), $crate::StatusCode> { + let status: i32 = parcel.read()?; + if status == $crate::binder_impl::NULL_PARCELABLE_FLAG { + Err($crate::StatusCode::UNEXPECTED_NULL) + } else { + use $crate::binder_impl::UnstructuredParcelable; + self.read_from_parcel(parcel) + } + } + } + + impl < $($param: Default),* > $crate::binder_impl::DeserializeArray for $parcelable < $($param),* > {} + + impl < $($param: Default),* > $crate::binder_impl::DeserializeOption for $parcelable < $($param),* > { + fn deserialize_option( + parcel: &$crate::binder_impl::BorrowedParcel<'_>, + ) -> std::result::Result<Option<Self>, $crate::StatusCode> { + let present: i32 = parcel.read()?; + match present { + $crate::binder_impl::NULL_PARCELABLE_FLAG => Ok(None), + $crate::binder_impl::NON_NULL_PARCELABLE_FLAG => { + use $crate::binder_impl::UnstructuredParcelable; + Ok(Some(Self::from_parcel(parcel)?)) + } + _ => Err(StatusCode::BAD_VALUE), + } + } + fn deserialize_option_from( + this: &mut Option<Self>, + parcel: &$crate::binder_impl::BorrowedParcel<'_>, + ) -> std::result::Result<(), $crate::StatusCode> { + let present: i32 = parcel.read()?; + match present { + $crate::binder_impl::NULL_PARCELABLE_FLAG => { + *this = None; + Ok(()) + } + $crate::binder_impl::NON_NULL_PARCELABLE_FLAG => { + use $crate::binder_impl::UnstructuredParcelable; + if let Some(this) = this { + this.read_from_parcel(parcel)?; + } else { + *this = Some(Self::from_parcel(parcel)?); + } + Ok(()) + } + _ => Err(StatusCode::BAD_VALUE), + } + } + } + }; +} + impl<T: Serialize> Serialize for Box<T> { fn serialize(&self, parcel: &mut BorrowedParcel<'_>) -> Result<()> { Serialize::serialize(&**self, parcel) diff --git a/libs/dumputils/dump_utils.cpp b/libs/dumputils/dump_utils.cpp index 5eb33087a2..f4cf11edb6 100644 --- a/libs/dumputils/dump_utils.cpp +++ b/libs/dumputils/dump_utils.cpp @@ -89,6 +89,9 @@ static const char* hidl_hal_interfaces_to_dump[] { /* list of hal interface to dump containing process during native dumps */ static const std::vector<std::string> aidl_interfaces_to_dump { + "android.hardware.audio.core.IConfig", + "android.hardware.audio.core.IModule", + "android.hardware.audio.effect.IFactory", "android.hardware.automotive.audiocontrol.IAudioControl", "android.hardware.automotive.can.ICanController", "android.hardware.automotive.evs.IEvsEnumerator", diff --git a/libs/gui/Surface.cpp b/libs/gui/Surface.cpp index 07a0cfed63..086544e48a 100644 --- a/libs/gui/Surface.cpp +++ b/libs/gui/Surface.cpp @@ -342,12 +342,23 @@ status_t Surface::getFrameTimestamps(uint64_t frameNumber, getFrameTimestamp(outRequestedPresentTime, events->requestedPresentTime); getFrameTimestamp(outLatchTime, events->latchTime); - getFrameTimestamp(outFirstRefreshStartTime, events->firstRefreshStartTime); + + nsecs_t firstRefreshStartTime = NATIVE_WINDOW_TIMESTAMP_INVALID; + getFrameTimestamp(&firstRefreshStartTime, events->firstRefreshStartTime); + if (outFirstRefreshStartTime) { + *outFirstRefreshStartTime = firstRefreshStartTime; + } + getFrameTimestamp(outLastRefreshStartTime, events->lastRefreshStartTime); getFrameTimestamp(outDequeueReadyTime, events->dequeueReadyTime); - getFrameTimestampFence(outAcquireTime, events->acquireFence, + nsecs_t acquireTime = NATIVE_WINDOW_TIMESTAMP_INVALID; + getFrameTimestampFence(&acquireTime, events->acquireFence, events->hasAcquireInfo()); + if (outAcquireTime != nullptr) { + *outAcquireTime = acquireTime; + } + getFrameTimestampFence(outGpuCompositionDoneTime, events->gpuCompositionDoneFence, events->hasGpuCompositionDoneInfo()); @@ -356,6 +367,16 @@ status_t Surface::getFrameTimestamps(uint64_t frameNumber, getFrameTimestampFence(outReleaseTime, events->releaseFence, events->hasReleaseInfo()); + // Fix up the GPU completion fence at this layer -- eglGetFrameTimestampsANDROID() expects + // that EGL_FIRST_COMPOSITION_GPU_FINISHED_TIME_ANDROID > EGL_RENDERING_COMPLETE_TIME_ANDROID. + // This is typically true, but SurfaceFlinger may opt to cache prior GPU composition results, + // which breaks that assumption, so zero out GPU composition time. + if (outGpuCompositionDoneTime != nullptr + && *outGpuCompositionDoneTime > 0 && (acquireTime > 0 || firstRefreshStartTime > 0) + && *outGpuCompositionDoneTime <= std::max(acquireTime, firstRefreshStartTime)) { + *outGpuCompositionDoneTime = 0; + } + return NO_ERROR; } diff --git a/libs/input/AccelerationCurve.cpp b/libs/input/AccelerationCurve.cpp new file mode 100644 index 0000000000..0a92a71596 --- /dev/null +++ b/libs/input/AccelerationCurve.cpp @@ -0,0 +1,63 @@ +/* + * Copyright 2024 The Android Open Source Project + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +#include <input/AccelerationCurve.h> + +#include <array> +#include <limits> + +#include <log/log_main.h> + +#define LOG_TAG "AccelerationCurve" + +namespace android { + +namespace { + +// The last segment must have an infinite maximum speed, so that all speeds are covered. +constexpr std::array<AccelerationCurveSegment, 4> kSegments = {{ + {32.002, 3.19, 0}, + {52.83, 4.79, -51.254}, + {119.124, 7.28, -182.737}, + {std::numeric_limits<double>::infinity(), 15.04, -1107.556}, +}}; + +static_assert(kSegments.back().maxPointerSpeedMmPerS == std::numeric_limits<double>::infinity()); + +constexpr std::array<double, 15> kSensitivityFactors = {1, 2, 4, 6, 7, 8, 9, 10, + 11, 12, 13, 14, 16, 18, 20}; + +} // namespace + +std::vector<AccelerationCurveSegment> createAccelerationCurveForPointerSensitivity( + int32_t sensitivity) { + LOG_ALWAYS_FATAL_IF(sensitivity < -7 || sensitivity > 7, "Invalid pointer sensitivity value"); + std::vector<AccelerationCurveSegment> output; + output.reserve(kSegments.size()); + + // The curves we want to produce for different sensitivity values are actually the same curve, + // just scaled in the Y (gain) axis by a sensitivity factor and a couple of constants. + double commonFactor = 0.64 * kSensitivityFactors[sensitivity + 7] / 10; + for (AccelerationCurveSegment seg : kSegments) { + output.push_back(AccelerationCurveSegment{seg.maxPointerSpeedMmPerS, + commonFactor * seg.baseGain, + commonFactor * seg.reciprocal}); + } + + return output; +} + +} // namespace android
\ No newline at end of file diff --git a/libs/input/Android.bp b/libs/input/Android.bp index b74c3b2e95..3d3bf1355f 100644 --- a/libs/input/Android.bp +++ b/libs/input/Android.bp @@ -175,6 +175,7 @@ cc_library { ], srcs: [ "android/os/IInputFlinger.aidl", + "AccelerationCurve.cpp", "Input.cpp", "InputDevice.cpp", "InputEventLabels.cpp", diff --git a/libs/input/InputTransport.cpp b/libs/input/InputTransport.cpp index e63b8f012f..37de00cf35 100644 --- a/libs/input/InputTransport.cpp +++ b/libs/input/InputTransport.cpp @@ -883,6 +883,9 @@ status_t InputConsumer::consume(InputEventFactoryInterface* factory, bool consum mConsumeTimes.emplace(mMsg.header.seq, systemTime(SYSTEM_TIME_MONOTONIC)); LOG_ALWAYS_FATAL_IF(!inserted, "Already have a consume time for seq=%" PRIu32, mMsg.header.seq); + + // Trace the event processing timeline - event was just read from the socket + ATRACE_ASYNC_BEGIN("InputConsumer processing", /*cookie=*/mMsg.header.seq); } if (result) { // Consume the next batched event unless batches are being held for later. @@ -1421,6 +1424,9 @@ status_t InputConsumer::sendUnchainedFinishedSignal(uint32_t seq, bool handled) // message anymore. If the socket write did not succeed, we will try again and will still // need consume time. popConsumeTime(seq); + + // Trace the event processing timeline - event was just finished + ATRACE_ASYNC_END("InputConsumer processing", /*cookie=*/seq); } return result; } diff --git a/libs/input/KeyLayoutMap.cpp b/libs/input/KeyLayoutMap.cpp index ab8c341b15..508818852e 100644 --- a/libs/input/KeyLayoutMap.cpp +++ b/libs/input/KeyLayoutMap.cpp @@ -97,6 +97,10 @@ static const std::unordered_map<std::string_view, InputDeviceSensorType> SENSOR_ bool kernelConfigsArePresent(const std::set<std::string>& configs) { #if defined(__ANDROID__) + if (configs.empty()) { + return true; + } + std::map<std::string, std::string> kernelConfigs; const status_t result = android::kernelconfigs::LoadKernelConfigs(&kernelConfigs); LOG_ALWAYS_FATAL_IF(result != OK, "Kernel configs could not be fetched"); diff --git a/libs/input/VelocityControl.cpp b/libs/input/VelocityControl.cpp index c835a081a5..edd31e91d3 100644 --- a/libs/input/VelocityControl.cpp +++ b/libs/input/VelocityControl.cpp @@ -15,7 +15,6 @@ */ #define LOG_TAG "VelocityControl" -//#define LOG_NDEBUG 0 // Log debug messages about acceleration. static constexpr bool DEBUG_ACCELERATION = false; @@ -23,6 +22,7 @@ static constexpr bool DEBUG_ACCELERATION = false; #include <math.h> #include <limits.h> +#include <android-base/logging.h> #include <input/VelocityControl.h> #include <utils/BitSet.h> #include <utils/Timers.h> @@ -37,15 +37,6 @@ VelocityControl::VelocityControl() { reset(); } -const VelocityControlParameters& VelocityControl::getParameters() const{ - return mParameters; -} - -void VelocityControl::setParameters(const VelocityControlParameters& parameters) { - mParameters = parameters; - reset(); -} - void VelocityControl::reset() { mLastMovementTime = LLONG_MIN; mRawPositionX = 0; @@ -54,65 +45,156 @@ void VelocityControl::reset() { } void VelocityControl::move(nsecs_t eventTime, float* deltaX, float* deltaY) { - if ((deltaX && *deltaX) || (deltaY && *deltaY)) { - if (eventTime >= mLastMovementTime + STOP_TIME) { - if (DEBUG_ACCELERATION && mLastMovementTime != LLONG_MIN) { - ALOGD("VelocityControl: stopped, last movement was %0.3fms ago", - (eventTime - mLastMovementTime) * 0.000001f); - } - reset(); - } + if ((deltaX == nullptr || *deltaX == 0) && (deltaY == nullptr || *deltaY == 0)) { + return; + } + if (eventTime >= mLastMovementTime + STOP_TIME) { + ALOGD_IF(DEBUG_ACCELERATION && mLastMovementTime != LLONG_MIN, + "VelocityControl: stopped, last movement was %0.3fms ago", + (eventTime - mLastMovementTime) * 0.000001f); + reset(); + } - mLastMovementTime = eventTime; - if (deltaX) { - mRawPositionX += *deltaX; - } - if (deltaY) { - mRawPositionY += *deltaY; - } - mVelocityTracker.addMovement(eventTime, /*pointerId=*/0, AMOTION_EVENT_AXIS_X, - mRawPositionX); - mVelocityTracker.addMovement(eventTime, /*pointerId=*/0, AMOTION_EVENT_AXIS_Y, - mRawPositionY); - - std::optional<float> vx = mVelocityTracker.getVelocity(AMOTION_EVENT_AXIS_X, 0); - std::optional<float> vy = mVelocityTracker.getVelocity(AMOTION_EVENT_AXIS_Y, 0); - float scale = mParameters.scale; - if (vx && vy) { - float speed = hypotf(*vx, *vy) * scale; - if (speed >= mParameters.highThreshold) { - // Apply full acceleration above the high speed threshold. - scale *= mParameters.acceleration; - } else if (speed > mParameters.lowThreshold) { - // Linearly interpolate the acceleration to apply between the low and high - // speed thresholds. - scale *= 1 + (speed - mParameters.lowThreshold) - / (mParameters.highThreshold - mParameters.lowThreshold) - * (mParameters.acceleration - 1); - } - - if (DEBUG_ACCELERATION) { - ALOGD("VelocityControl(%0.3f, %0.3f, %0.3f, %0.3f): " - "vx=%0.3f, vy=%0.3f, speed=%0.3f, accel=%0.3f", - mParameters.scale, mParameters.lowThreshold, mParameters.highThreshold, - mParameters.acceleration, *vx, *vy, speed, scale / mParameters.scale); - } - - } else { - if (DEBUG_ACCELERATION) { - ALOGD("VelocityControl(%0.3f, %0.3f, %0.3f, %0.3f): unknown velocity", - mParameters.scale, mParameters.lowThreshold, mParameters.highThreshold, - mParameters.acceleration); - } - } + mLastMovementTime = eventTime; + if (deltaX) { + mRawPositionX += *deltaX; + } + if (deltaY) { + mRawPositionY += *deltaY; + } + mVelocityTracker.addMovement(eventTime, /*pointerId=*/0, AMOTION_EVENT_AXIS_X, mRawPositionX); + mVelocityTracker.addMovement(eventTime, /*pointerId=*/0, AMOTION_EVENT_AXIS_Y, mRawPositionY); + scaleDeltas(deltaX, deltaY); +} + +// --- SimpleVelocityControl --- + +const VelocityControlParameters& SimpleVelocityControl::getParameters() const { + return mParameters; +} - if (deltaX) { - *deltaX *= scale; +void SimpleVelocityControl::setParameters(const VelocityControlParameters& parameters) { + mParameters = parameters; + reset(); +} + +void SimpleVelocityControl::scaleDeltas(float* deltaX, float* deltaY) { + std::optional<float> vx = mVelocityTracker.getVelocity(AMOTION_EVENT_AXIS_X, 0); + std::optional<float> vy = mVelocityTracker.getVelocity(AMOTION_EVENT_AXIS_Y, 0); + float scale = mParameters.scale; + if (vx.has_value() && vy.has_value()) { + float speed = hypotf(*vx, *vy) * scale; + if (speed >= mParameters.highThreshold) { + // Apply full acceleration above the high speed threshold. + scale *= mParameters.acceleration; + } else if (speed > mParameters.lowThreshold) { + // Linearly interpolate the acceleration to apply between the low and high + // speed thresholds. + scale *= 1 + + (speed - mParameters.lowThreshold) / + (mParameters.highThreshold - mParameters.lowThreshold) * + (mParameters.acceleration - 1); } - if (deltaY) { - *deltaY *= scale; + + ALOGD_IF(DEBUG_ACCELERATION, + "SimpleVelocityControl(%0.3f, %0.3f, %0.3f, %0.3f): " + "vx=%0.3f, vy=%0.3f, speed=%0.3f, accel=%0.3f", + mParameters.scale, mParameters.lowThreshold, mParameters.highThreshold, + mParameters.acceleration, *vx, *vy, speed, scale / mParameters.scale); + + } else { + ALOGD_IF(DEBUG_ACCELERATION, + "SimpleVelocityControl(%0.3f, %0.3f, %0.3f, %0.3f): unknown velocity", + mParameters.scale, mParameters.lowThreshold, mParameters.highThreshold, + mParameters.acceleration); + } + + if (deltaX != nullptr) { + *deltaX *= scale; + } + if (deltaY != nullptr) { + *deltaY *= scale; + } +} + +// --- CurvedVelocityControl --- + +namespace { + +/** + * The resolution that we assume a mouse to have, in counts per inch. + * + * Mouse resolutions vary wildly, but 800 CPI is probably the most common. There should be enough + * range in the available sensitivity settings to accommodate users of mice with other resolutions. + */ +constexpr int32_t MOUSE_CPI = 800; + +float countsToMm(float counts) { + return counts / MOUSE_CPI * 25.4; +} + +} // namespace + +CurvedVelocityControl::CurvedVelocityControl() + : mCurveSegments(createAccelerationCurveForPointerSensitivity(0)) {} + +void CurvedVelocityControl::setCurve(const std::vector<AccelerationCurveSegment>& curve) { + mCurveSegments = curve; +} + +void CurvedVelocityControl::setAccelerationEnabled(bool enabled) { + mAccelerationEnabled = enabled; +} + +void CurvedVelocityControl::scaleDeltas(float* deltaX, float* deltaY) { + if (!mAccelerationEnabled) { + ALOGD_IF(DEBUG_ACCELERATION, "CurvedVelocityControl: acceleration disabled"); + return; + } + + std::optional<float> vx = mVelocityTracker.getVelocity(AMOTION_EVENT_AXIS_X, 0); + std::optional<float> vy = mVelocityTracker.getVelocity(AMOTION_EVENT_AXIS_Y, 0); + + float ratio; + if (vx.has_value() && vy.has_value()) { + float vxMmPerS = countsToMm(*vx); + float vyMmPerS = countsToMm(*vy); + float speedMmPerS = sqrtf(vxMmPerS * vxMmPerS + vyMmPerS * vyMmPerS); + + const AccelerationCurveSegment& seg = segmentForSpeed(speedMmPerS); + ratio = seg.baseGain + seg.reciprocal / speedMmPerS; + ALOGD_IF(DEBUG_ACCELERATION, + "CurvedVelocityControl: velocities (%0.3f, %0.3f) → speed %0.3f → ratio %0.3f", + vxMmPerS, vyMmPerS, speedMmPerS, ratio); + } else { + // We don't have enough data to compute a velocity yet. This happens early in the movement, + // when the speed is presumably low, so use the base gain of the first segment of the curve. + // (This would behave oddly for curves with a reciprocal term on the first segment, but we + // don't have any of those, and they'd be very strange at velocities close to zero anyway.) + ratio = mCurveSegments[0].baseGain; + ALOGD_IF(DEBUG_ACCELERATION, + "CurvedVelocityControl: unknown velocity, using base gain of first segment (%.3f)", + ratio); + } + + if (deltaX != nullptr) { + *deltaX *= ratio; + } + if (deltaY != nullptr) { + *deltaY *= ratio; + } +} + +const AccelerationCurveSegment& CurvedVelocityControl::segmentForSpeed(float speedMmPerS) { + for (const AccelerationCurveSegment& seg : mCurveSegments) { + if (speedMmPerS <= seg.maxPointerSpeedMmPerS) { + return seg; } } + ALOGE("CurvedVelocityControl: No segment found for speed %.3f; last segment should always have " + "a max speed of infinity.", + speedMmPerS); + return mCurveSegments.back(); } } // namespace android diff --git a/libs/input/input_flags.aconfig b/libs/input/input_flags.aconfig index 11f69941bd..1baeb26879 100644 --- a/libs/input/input_flags.aconfig +++ b/libs/input/input_flags.aconfig @@ -97,3 +97,10 @@ flag { description: "Remove pointer event tracking in WM after the Pointer Icon Refactor" bug: "315321016" } + +flag { + name: "enable_new_mouse_pointer_ballistics" + namespace: "input" + description: "Change the acceleration curves for mouse pointer movements to match the touchpad ones" + bug: "315313622" +} diff --git a/libs/input/tests/Android.bp b/libs/input/tests/Android.bp index 13cfb491b5..0485ff6e1e 100644 --- a/libs/input/tests/Android.bp +++ b/libs/input/tests/Android.bp @@ -25,6 +25,7 @@ cc_test { "TfLiteMotionPredictor_test.cpp", "TouchResampling_test.cpp", "TouchVideoFrame_test.cpp", + "VelocityControl_test.cpp", "VelocityTracker_test.cpp", "VerifiedInputEvent_test.cpp", ], diff --git a/libs/input/tests/VelocityControl_test.cpp b/libs/input/tests/VelocityControl_test.cpp new file mode 100644 index 0000000000..63d64c6048 --- /dev/null +++ b/libs/input/tests/VelocityControl_test.cpp @@ -0,0 +1,129 @@ +/* + * Copyright 2024 The Android Open Source Project + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +#include <input/VelocityControl.h> + +#include <limits> + +#include <gtest/gtest.h> +#include <input/AccelerationCurve.h> +#include <utils/Timers.h> + +namespace android { + +namespace { + +constexpr float EPSILON = 0.001; +constexpr float COUNTS_PER_MM = 800 / 25.4; + +} // namespace + +class CurvedVelocityControlTest : public testing::Test { +protected: + CurvedVelocityControl mCtrl; + + void moveWithoutCheckingResult(nsecs_t eventTime, float deltaX, float deltaY) { + mCtrl.move(eventTime, &deltaX, &deltaY); + } + + void moveAndCheckRatio(nsecs_t eventTime, const float deltaX, const float deltaY, + float expectedRatio) { + float newDeltaX = deltaX, newDeltaY = deltaY; + mCtrl.move(eventTime, &newDeltaX, &newDeltaY); + ASSERT_NEAR(expectedRatio * deltaX, newDeltaX, EPSILON) + << "Expected ratio of " << expectedRatio << " in X, but actual ratio was " + << newDeltaX / deltaX; + ASSERT_NEAR(expectedRatio * deltaY, newDeltaY, EPSILON) + << "Expected ratio of " << expectedRatio << " in Y, but actual ratio was " + << newDeltaY / deltaY; + } +}; + +TEST_F(CurvedVelocityControlTest, SegmentSelection) { + // To make the maths simple, use a "curve" that's actually just a sequence of steps. + mCtrl.setCurve({ + {10, 2, 0}, + {20, 3, 0}, + {30, 4, 0}, + {std::numeric_limits<double>::infinity(), 5, 0}, + }); + + // Establish a velocity of 16 mm/s. + moveWithoutCheckingResult(0, 0, 0); + moveWithoutCheckingResult(10'000'000, 0.16 * COUNTS_PER_MM, 0); + moveWithoutCheckingResult(20'000'000, 0.16 * COUNTS_PER_MM, 0); + moveWithoutCheckingResult(30'000'000, 0.16 * COUNTS_PER_MM, 0); + ASSERT_NO_FATAL_FAILURE( + moveAndCheckRatio(40'000'000, 0.16 * COUNTS_PER_MM, 0, /*expectedRatio=*/3)); + + // Establish a velocity of 50 mm/s. + mCtrl.reset(); + moveWithoutCheckingResult(100'000'000, 0, 0); + moveWithoutCheckingResult(110'000'000, 0.50 * COUNTS_PER_MM, 0); + moveWithoutCheckingResult(120'000'000, 0.50 * COUNTS_PER_MM, 0); + moveWithoutCheckingResult(130'000'000, 0.50 * COUNTS_PER_MM, 0); + ASSERT_NO_FATAL_FAILURE( + moveAndCheckRatio(140'000'000, 0.50 * COUNTS_PER_MM, 0, /*expectedRatio=*/5)); +} + +TEST_F(CurvedVelocityControlTest, RatioDefaultsToFirstSegmentWhenVelocityIsUnknown) { + mCtrl.setCurve({ + {10, 3, 0}, + {20, 2, 0}, + {std::numeric_limits<double>::infinity(), 4, 0}, + }); + + // Only send two moves, which won't be enough for VelocityTracker to calculate a velocity from. + moveWithoutCheckingResult(0, 0, 0); + ASSERT_NO_FATAL_FAILURE( + moveAndCheckRatio(10'000'000, 0.25 * COUNTS_PER_MM, 0, /*expectedRatio=*/3)); +} + +TEST_F(CurvedVelocityControlTest, VelocityCalculatedUsingBothAxes) { + mCtrl.setCurve({ + {8.0, 3, 0}, + {8.1, 2, 0}, + {std::numeric_limits<double>::infinity(), 4, 0}, + }); + + // Establish a velocity of 8.06 (= √65 = √(7²+4²)) mm/s between the two axes. + moveWithoutCheckingResult(0, 0, 0); + moveWithoutCheckingResult(10'000'000, 0.07 * COUNTS_PER_MM, 0.04 * COUNTS_PER_MM); + moveWithoutCheckingResult(20'000'000, 0.07 * COUNTS_PER_MM, 0.04 * COUNTS_PER_MM); + moveWithoutCheckingResult(30'000'000, 0.07 * COUNTS_PER_MM, 0.04 * COUNTS_PER_MM); + ASSERT_NO_FATAL_FAILURE(moveAndCheckRatio(40'000'000, 0.07 * COUNTS_PER_MM, + 0.04 * COUNTS_PER_MM, + /*expectedRatio=*/2)); +} + +TEST_F(CurvedVelocityControlTest, ReciprocalTerm) { + mCtrl.setCurve({ + {10, 2, 0}, + {20, 3, -10}, + {std::numeric_limits<double>::infinity(), 3, 0}, + }); + + // Establish a velocity of 15 mm/s. + moveWithoutCheckingResult(0, 0, 0); + moveWithoutCheckingResult(10'000'000, 0, 0.15 * COUNTS_PER_MM); + moveWithoutCheckingResult(20'000'000, 0, 0.15 * COUNTS_PER_MM); + moveWithoutCheckingResult(30'000'000, 0, 0.15 * COUNTS_PER_MM); + // Expected ratio is 3 - 10 / 15 = 2.33333... + ASSERT_NO_FATAL_FAILURE( + moveAndCheckRatio(40'000'000, 0, 0.15 * COUNTS_PER_MM, /*expectedRatio=*/2.33333)); +} + +} // namespace android
\ No newline at end of file diff --git a/libs/nativewindow/rust/src/lib.rs b/libs/nativewindow/rust/src/lib.rs index 6f86c4a48f..aab7df0c2c 100644 --- a/libs/nativewindow/rust/src/lib.rs +++ b/libs/nativewindow/rust/src/lib.rs @@ -19,10 +19,8 @@ extern crate nativewindow_bindgen as ffi; pub use ffi::{AHardwareBuffer_Format, AHardwareBuffer_UsageFlags}; use binder::{ - binder_impl::{ - BorrowedParcel, Deserialize, DeserializeArray, DeserializeOption, Serialize, - SerializeArray, SerializeOption, NON_NULL_PARCELABLE_FLAG, NULL_PARCELABLE_FLAG, - }, + binder_impl::{BorrowedParcel, UnstructuredParcelable}, + impl_deserialize_for_unstructured_parcelable, impl_serialize_for_unstructured_parcelable, unstable_api::{status_result, AsNative}, StatusCode, }; @@ -102,20 +100,35 @@ impl HardwareBuffer { } } - /// Adopts the raw pointer and wraps it in a Rust AHardwareBuffer. - /// - /// # Errors - /// - /// Will panic if buffer_ptr is null. + /// Adopts the given raw pointer and wraps it in a Rust HardwareBuffer. /// /// # Safety /// - /// This function adopts the pointer but does NOT increment the refcount on the buffer. If the - /// caller uses the pointer after the created object is dropped it will cause a memory leak. + /// This function takes ownership of the pointer and does NOT increment the refcount on the + /// buffer. If the caller uses the pointer after the created object is dropped it will cause + /// undefined behaviour. If the caller wants to continue using the pointer after calling this + /// then use [`clone_from_raw`](Self::clone_from_raw) instead. pub unsafe fn from_raw(buffer_ptr: NonNull<AHardwareBuffer>) -> Self { Self(buffer_ptr) } + /// Creates a new Rust HardwareBuffer to wrap the given AHardwareBuffer without taking ownership + /// of it. + /// + /// Unlike [`from_raw`](Self::from_raw) this method will increment the refcount on the buffer. + /// This means that the caller can continue to use the raw buffer it passed in, and must call + /// [`AHardwareBuffer_release`](ffi::AHardwareBuffer_release) when it is finished with it to + /// avoid a memory leak. + /// + /// # Safety + /// + /// The buffer pointer must point to a valid `AHardwareBuffer`. + pub unsafe fn clone_from_raw(buffer: NonNull<AHardwareBuffer>) -> Self { + // SAFETY: The caller guarantees that the AHardwareBuffer pointer is valid. + unsafe { ffi::AHardwareBuffer_acquire(buffer.as_ptr()) }; + Self(buffer) + } + /// Get the internal |AHardwareBuffer| pointer without decrementing the refcount. This can /// be used to provide a pointer to the AHB for a C/C++ API over the FFI. pub fn into_raw(self) -> NonNull<AHardwareBuffer> { @@ -210,81 +223,40 @@ impl Clone for HardwareBuffer { } } -impl Serialize for HardwareBuffer { - fn serialize(&self, parcel: &mut BorrowedParcel) -> Result<(), StatusCode> { - SerializeOption::serialize_option(Some(self), parcel) - } -} - -impl SerializeOption for HardwareBuffer { - fn serialize_option( - this: Option<&Self>, - parcel: &mut BorrowedParcel, - ) -> Result<(), StatusCode> { - if let Some(this) = this { - parcel.write(&NON_NULL_PARCELABLE_FLAG)?; - - let status = - // SAFETY: The AHardwareBuffer pointer we pass is guaranteed to be non-null and valid - // because it must have been allocated by `AHardwareBuffer_allocate`, - // `AHardwareBuffer_readFromParcel` or the caller of `from_raw` and we have not yet - // released it. - unsafe { AHardwareBuffer_writeToParcel(this.0.as_ptr(), parcel.as_native_mut()) }; - status_result(status) - } else { - parcel.write(&NULL_PARCELABLE_FLAG) - } +impl UnstructuredParcelable for HardwareBuffer { + fn write_to_parcel(&self, parcel: &mut BorrowedParcel) -> Result<(), StatusCode> { + let status = + // SAFETY: The AHardwareBuffer pointer we pass is guaranteed to be non-null and valid + // because it must have been allocated by `AHardwareBuffer_allocate`, + // `AHardwareBuffer_readFromParcel` or the caller of `from_raw` and we have not yet + // released it. + unsafe { AHardwareBuffer_writeToParcel(self.0.as_ptr(), parcel.as_native_mut()) }; + status_result(status) } -} - -impl Deserialize for HardwareBuffer { - type UninitType = Option<Self>; - fn uninit() -> Option<Self> { - None - } + fn from_parcel(parcel: &BorrowedParcel) -> Result<Self, StatusCode> { + let mut buffer = null_mut(); - fn from_init(value: Self) -> Option<Self> { - Some(value) - } + let status = + // SAFETY: Both pointers must be valid because they are obtained from references. + // `AHardwareBuffer_readFromParcel` doesn't store them or do anything else special + // with them. If it returns success then it will have allocated a new + // `AHardwareBuffer` and incremented the reference count, so we can use it until we + // release it. + unsafe { AHardwareBuffer_readFromParcel(parcel.as_native(), &mut buffer) }; - fn deserialize(parcel: &BorrowedParcel) -> Result<Self, StatusCode> { - DeserializeOption::deserialize_option(parcel) - .transpose() - .unwrap_or(Err(StatusCode::UNEXPECTED_NULL)) - } -} + status_result(status)?; -impl DeserializeOption for HardwareBuffer { - fn deserialize_option(parcel: &BorrowedParcel) -> Result<Option<Self>, StatusCode> { - let present: i32 = parcel.read()?; - match present { - NULL_PARCELABLE_FLAG => Ok(None), - NON_NULL_PARCELABLE_FLAG => { - let mut buffer = null_mut(); - - let status = - // SAFETY: Both pointers must be valid because they are obtained from references. - // `AHardwareBuffer_readFromParcel` doesn't store them or do anything else special - // with them. If it returns success then it will have allocated a new - // `AHardwareBuffer` and incremented the reference count, so we can use it until we - // release it. - unsafe { AHardwareBuffer_readFromParcel(parcel.as_native(), &mut buffer) }; - - status_result(status)?; - - Ok(Some(Self(NonNull::new(buffer).expect( - "AHardwareBuffer_readFromParcel returned success but didn't allocate buffer", - )))) - } - _ => Err(StatusCode::BAD_VALUE), - } + Ok(Self( + NonNull::new(buffer).expect( + "AHardwareBuffer_readFromParcel returned success but didn't allocate buffer", + ), + )) } } -impl SerializeArray for HardwareBuffer {} - -impl DeserializeArray for HardwareBuffer {} +impl_deserialize_for_unstructured_parcelable!(HardwareBuffer); +impl_serialize_for_unstructured_parcelable!(HardwareBuffer); // SAFETY: The underlying *AHardwareBuffers can be moved between threads. unsafe impl Send for HardwareBuffer {} diff --git a/services/inputflinger/include/InputReaderBase.h b/services/inputflinger/include/InputReaderBase.h index efc8b260f3..40359a42c9 100644 --- a/services/inputflinger/include/InputReaderBase.h +++ b/services/inputflinger/include/InputReaderBase.h @@ -126,7 +126,20 @@ struct InputReaderConfiguration { // The suggested display ID to show the cursor. int32_t defaultPointerDisplayId; + // The mouse pointer speed, as a number from -7 (slowest) to 7 (fastest). + // + // Currently only used when the enable_new_mouse_pointer_ballistics flag is enabled. + int32_t mousePointerSpeed; + + // Whether to apply an acceleration curve to pointer movements from mice. + // + // Currently only used when the enable_new_mouse_pointer_ballistics flag is enabled. + bool mousePointerAccelerationEnabled; + // Velocity control parameters for mouse pointer movements. + // + // If the enable_new_mouse_pointer_ballistics flag is enabled, these are ignored and the values + // of mousePointerSpeed and mousePointerAccelerationEnabled used instead. VelocityControlParameters pointerVelocityControlParameters; // Velocity control parameters for mouse wheel movements. @@ -229,6 +242,8 @@ struct InputReaderConfiguration { InputReaderConfiguration() : virtualKeyQuietTime(0), + mousePointerSpeed(0), + mousePointerAccelerationEnabled(true), pointerVelocityControlParameters(1.0f, 500.0f, 3000.0f, static_cast<float>( android::os::IInputConstants:: diff --git a/services/inputflinger/reader/mapper/CursorInputMapper.cpp b/services/inputflinger/reader/mapper/CursorInputMapper.cpp index 58e35a6aba..4cebd646db 100644 --- a/services/inputflinger/reader/mapper/CursorInputMapper.cpp +++ b/services/inputflinger/reader/mapper/CursorInputMapper.cpp @@ -20,9 +20,11 @@ #include "CursorInputMapper.h" -#include <com_android_input_flags.h> #include <optional> +#include <com_android_input_flags.h> +#include <input/AccelerationCurve.h> + #include "CursorButtonAccumulator.h" #include "CursorScrollAccumulator.h" #include "PointerControllerInterface.h" @@ -75,7 +77,8 @@ CursorInputMapper::CursorInputMapper(InputDeviceContext& deviceContext, const InputReaderConfiguration& readerConfig) : InputMapper(deviceContext, readerConfig), mLastEventTime(std::numeric_limits<nsecs_t>::min()), - mEnablePointerChoreographer(input_flags::enable_pointer_choreographer()) {} + mEnablePointerChoreographer(input_flags::enable_pointer_choreographer()), + mEnableNewMousePointerBallistics(input_flags::enable_new_mouse_pointer_ballistics()) {} CursorInputMapper::~CursorInputMapper() { if (mPointerController != nullptr) { @@ -204,7 +207,8 @@ std::list<NotifyArgs> CursorInputMapper::reset(nsecs_t when) { mDownTime = 0; mLastEventTime = std::numeric_limits<nsecs_t>::min(); - mPointerVelocityControl.reset(); + mOldPointerVelocityControl.reset(); + mNewPointerVelocityControl.reset(); mWheelXVelocityControl.reset(); mWheelYVelocityControl.reset(); @@ -282,7 +286,11 @@ std::list<NotifyArgs> CursorInputMapper::sync(nsecs_t when, nsecs_t readTime) { mWheelYVelocityControl.move(when, nullptr, &vscroll); mWheelXVelocityControl.move(when, &hscroll, nullptr); - mPointerVelocityControl.move(when, &deltaX, &deltaY); + if (mEnableNewMousePointerBallistics) { + mNewPointerVelocityControl.move(when, &deltaX, &deltaY); + } else { + mOldPointerVelocityControl.move(when, &deltaX, &deltaY); + } float xCursorPosition = AMOTION_EVENT_INVALID_CURSOR_POSITION; float yCursorPosition = AMOTION_EVENT_INVALID_CURSOR_POSITION; @@ -492,11 +500,22 @@ void CursorInputMapper::configureOnPointerCapture(const InputReaderConfiguration void CursorInputMapper::configureOnChangePointerSpeed(const InputReaderConfiguration& config) { if (mParameters.mode == Parameters::Mode::POINTER_RELATIVE) { // Disable any acceleration or scaling for the pointer when Pointer Capture is enabled. - mPointerVelocityControl.setParameters(FLAT_VELOCITY_CONTROL_PARAMS); + if (mEnableNewMousePointerBallistics) { + mNewPointerVelocityControl.setAccelerationEnabled(false); + } else { + mOldPointerVelocityControl.setParameters(FLAT_VELOCITY_CONTROL_PARAMS); + } mWheelXVelocityControl.setParameters(FLAT_VELOCITY_CONTROL_PARAMS); mWheelYVelocityControl.setParameters(FLAT_VELOCITY_CONTROL_PARAMS); } else { - mPointerVelocityControl.setParameters(config.pointerVelocityControlParameters); + if (mEnableNewMousePointerBallistics) { + mNewPointerVelocityControl.setAccelerationEnabled( + config.mousePointerAccelerationEnabled); + mNewPointerVelocityControl.setCurve( + createAccelerationCurveForPointerSensitivity(config.mousePointerSpeed)); + } else { + mOldPointerVelocityControl.setParameters(config.pointerVelocityControlParameters); + } mWheelXVelocityControl.setParameters(config.wheelVelocityControlParameters); mWheelYVelocityControl.setParameters(config.wheelVelocityControlParameters); } diff --git a/services/inputflinger/reader/mapper/CursorInputMapper.h b/services/inputflinger/reader/mapper/CursorInputMapper.h index 308adaa463..1ddf6f2b5b 100644 --- a/services/inputflinger/reader/mapper/CursorInputMapper.h +++ b/services/inputflinger/reader/mapper/CursorInputMapper.h @@ -26,7 +26,6 @@ namespace android { -class VelocityControl; class PointerControllerInterface; class CursorButtonAccumulator; @@ -111,9 +110,10 @@ private: // Velocity controls for mouse pointer and wheel movements. // The controls for X and Y wheel movements are separate to keep them decoupled. - VelocityControl mPointerVelocityControl; - VelocityControl mWheelXVelocityControl; - VelocityControl mWheelYVelocityControl; + SimpleVelocityControl mOldPointerVelocityControl; + CurvedVelocityControl mNewPointerVelocityControl; + SimpleVelocityControl mWheelXVelocityControl; + SimpleVelocityControl mWheelYVelocityControl; // The display that events generated by this mapper should target. This can be set to // ADISPLAY_ID_NONE to target the focused display. If there is no display target (i.e. @@ -129,6 +129,7 @@ private: nsecs_t mLastEventTime; const bool mEnablePointerChoreographer; + const bool mEnableNewMousePointerBallistics; explicit CursorInputMapper(InputDeviceContext& deviceContext, const InputReaderConfiguration& readerConfig); diff --git a/services/inputflinger/reader/mapper/TouchInputMapper.h b/services/inputflinger/reader/mapper/TouchInputMapper.h index bd9371d263..4b39e4099c 100644 --- a/services/inputflinger/reader/mapper/TouchInputMapper.h +++ b/services/inputflinger/reader/mapper/TouchInputMapper.h @@ -708,9 +708,9 @@ private: } mPointerSimple; // The pointer and scroll velocity controls. - VelocityControl mPointerVelocityControl; - VelocityControl mWheelXVelocityControl; - VelocityControl mWheelYVelocityControl; + SimpleVelocityControl mPointerVelocityControl; + SimpleVelocityControl mWheelXVelocityControl; + SimpleVelocityControl mWheelYVelocityControl; std::optional<DisplayViewport> findViewport(); diff --git a/services/inputflinger/reader/mapper/TouchpadInputMapper.cpp b/services/inputflinger/reader/mapper/TouchpadInputMapper.cpp index 6f697dbc31..bdc164029c 100644 --- a/services/inputflinger/reader/mapper/TouchpadInputMapper.cpp +++ b/services/inputflinger/reader/mapper/TouchpadInputMapper.cpp @@ -29,6 +29,7 @@ #include <android/input.h> #include <com_android_input_flags.h> #include <ftl/enum.h> +#include <input/AccelerationCurve.h> #include <input/PrintTools.h> #include <linux/input-event-codes.h> #include <log/log_main.h> @@ -55,27 +56,10 @@ const bool DEBUG_TOUCHPAD_GESTURES = __android_log_is_loggable(ANDROID_LOG_DEBUG, "TouchpadInputMapperGestures", ANDROID_LOG_INFO); -// Describes a segment of the acceleration curve. -struct CurveSegment { - // The maximum pointer speed which this segment should apply. The last segment in a curve should - // always set this to infinity. - double maxPointerSpeedMmPerS; - double slope; - double intercept; -}; - -const std::vector<CurveSegment> segments = { - {32.002, 3.19, 0}, - {52.83, 4.79, -51.254}, - {119.124, 7.28, -182.737}, - {std::numeric_limits<double>::infinity(), 15.04, -1107.556}, -}; - -const std::vector<double> sensitivityFactors = {1, 2, 4, 6, 7, 8, 9, 10, - 11, 12, 13, 14, 16, 18, 20}; - std::vector<double> createAccelerationCurveForSensitivity(int32_t sensitivity, size_t propertySize) { + std::vector<AccelerationCurveSegment> segments = + createAccelerationCurveForPointerSensitivity(sensitivity); LOG_ALWAYS_FATAL_IF(propertySize < 4 * segments.size()); std::vector<double> output(propertySize, 0); @@ -85,31 +69,23 @@ std::vector<double> createAccelerationCurveForSensitivity(int32_t sensitivity, // // (a, b, and c are also called sqr_, mul_, and int_ in the Gestures library code.) // - // We are trying to implement the following function, where slope and intercept are the - // parameters specified in the `segments` array above: - // gain(input_speed_mm) = - // 0.64 * (sensitivityFactor / 10) * (slope + intercept / input_speed_mm) + // createAccelerationCurveForPointerSensitivity gives us parameters for a function of the form: + // gain(input_speed_mm) = baseGain + reciprocal / input_speed_mm // Where "gain" is a multiplier applied to the input speed to produce the output speed: // output_speed(input_speed_mm) = input_speed_mm * gain(input_speed_mm) // // To put our function in the library's form, we substitute it into the function above: - // output_speed(input_speed_mm) = - // input_speed_mm * (0.64 * (sensitivityFactor / 10) * - // (slope + 25.4 * intercept / input_speed_mm)) - // then expand the brackets so that input_speed_mm cancels out for the intercept term: - // gain(input_speed_mm) = - // 0.64 * (sensitivityFactor / 10) * slope * input_speed_mm + - // 0.64 * (sensitivityFactor / 10) * intercept + // output_speed(input_speed_mm) = input_speed_mm * (baseGain + reciprocal / input_speed_mm) + // then expand the brackets so that input_speed_mm cancels out for the reciprocal term: + // gain(input_speed_mm) = baseGain * input_speed_mm + reciprocal // // This gives us the following parameters for the Gestures library function form: // a = 0 - // b = 0.64 * (sensitivityFactor / 10) * slope - // c = 0.64 * (sensitivityFactor / 10) * intercept - - double commonFactor = 0.64 * sensitivityFactors[sensitivity + 7] / 10; + // b = baseGain + // c = reciprocal size_t i = 0; - for (CurveSegment seg : segments) { + for (AccelerationCurveSegment seg : segments) { // The library's curve format consists of four doubles per segment: // * maximum pointer speed for the segment (mm/s) // * multiplier for the x² term (a.k.a. "a" or "sqr") @@ -118,8 +94,8 @@ std::vector<double> createAccelerationCurveForSensitivity(int32_t sensitivity, // (see struct CurveSegment in the library's AccelFilterInterpreter) output[i + 0] = seg.maxPointerSpeedMmPerS; output[i + 1] = 0; - output[i + 2] = commonFactor * seg.slope; - output[i + 3] = commonFactor * seg.intercept; + output[i + 2] = seg.baseGain; + output[i + 3] = seg.reciprocal; i += 4; } diff --git a/services/inputflinger/tests/CursorInputMapper_test.cpp b/services/inputflinger/tests/CursorInputMapper_test.cpp index 66c3256815..e104543523 100644 --- a/services/inputflinger/tests/CursorInputMapper_test.cpp +++ b/services/inputflinger/tests/CursorInputMapper_test.cpp @@ -193,6 +193,7 @@ class CursorInputMapperUnitTest : public CursorInputMapperUnitTestBase { protected: void SetUp() override { input_flags::enable_pointer_choreographer(false); + input_flags::enable_new_mouse_pointer_ballistics(false); CursorInputMapperUnitTestBase::SetUp(); } }; @@ -954,6 +955,7 @@ class CursorInputMapperUnitTestWithChoreographer : public CursorInputMapperUnitT protected: void SetUp() override { input_flags::enable_pointer_choreographer(true); + input_flags::enable_new_mouse_pointer_ballistics(false); CursorInputMapperUnitTestBase::SetUp(); } }; @@ -1280,6 +1282,48 @@ TEST_F(CursorInputMapperUnitTestWithChoreographer, ConfigureDisplayIdNoAssociate WithCoords(0.0f, 0.0f))))); } +// TODO(b/320433834): De-duplicate the test cases once the flag is removed. +class CursorInputMapperUnitTestWithNewBallistics : public CursorInputMapperUnitTestBase { +protected: + void SetUp() override { + input_flags::enable_pointer_choreographer(true); + input_flags::enable_new_mouse_pointer_ballistics(true); + CursorInputMapperUnitTestBase::SetUp(); + } +}; + +TEST_F(CursorInputMapperUnitTestWithNewBallistics, PointerCaptureDisablesVelocityProcessing) { + mPropertyMap.addProperty("cursor.mode", "pointer"); + createMapper(); + + NotifyMotionArgs motionArgs; + std::list<NotifyArgs> args; + + // Move and verify scale is applied. + args += process(ARBITRARY_TIME, EV_REL, REL_X, 10); + args += process(ARBITRARY_TIME, EV_REL, REL_Y, 20); + args += process(ARBITRARY_TIME, EV_SYN, SYN_REPORT, 0); + motionArgs = std::get<NotifyMotionArgs>(args.front()); + const float relX = motionArgs.pointerCoords[0].getAxisValue(AMOTION_EVENT_AXIS_RELATIVE_X); + const float relY = motionArgs.pointerCoords[0].getAxisValue(AMOTION_EVENT_AXIS_RELATIVE_Y); + ASSERT_GT(relX, 10); + ASSERT_GT(relY, 20); + args.clear(); + + // Enable Pointer Capture + setPointerCapture(true); + + // Move and verify scale is not applied. + args += process(ARBITRARY_TIME, EV_REL, REL_X, 10); + args += process(ARBITRARY_TIME, EV_REL, REL_Y, 20); + args += process(ARBITRARY_TIME, EV_SYN, SYN_REPORT, 0); + motionArgs = std::get<NotifyMotionArgs>(args.front()); + const float relX2 = motionArgs.pointerCoords[0].getAxisValue(AMOTION_EVENT_AXIS_RELATIVE_X); + const float relY2 = motionArgs.pointerCoords[0].getAxisValue(AMOTION_EVENT_AXIS_RELATIVE_Y); + ASSERT_EQ(10, relX2); + ASSERT_EQ(20, relY2); +} + namespace { // Minimum timestamp separation between subsequent input events from a Bluetooth device. diff --git a/services/surfaceflinger/Display/DisplayModeRequest.h b/services/surfaceflinger/Display/DisplayModeRequest.h index d07cdf55d2..c0e77bb5fc 100644 --- a/services/surfaceflinger/Display/DisplayModeRequest.h +++ b/services/surfaceflinger/Display/DisplayModeRequest.h @@ -27,6 +27,9 @@ struct DisplayModeRequest { // Whether to emit DisplayEventReceiver::DISPLAY_EVENT_MODE_CHANGE. bool emitEvent = false; + + // Whether to force the request to be applied, even if the mode is unchanged. + bool force = false; }; inline bool operator==(const DisplayModeRequest& lhs, const DisplayModeRequest& rhs) { diff --git a/services/surfaceflinger/DisplayDevice.cpp b/services/surfaceflinger/DisplayDevice.cpp index 799d62c19d..b96264b65e 100644 --- a/services/surfaceflinger/DisplayDevice.cpp +++ b/services/surfaceflinger/DisplayDevice.cpp @@ -24,6 +24,7 @@ #define ATRACE_TAG ATRACE_TAG_GRAPHICS +#include <common/FlagManager.h> #include <compositionengine/CompositionEngine.h> #include <compositionengine/Display.h> #include <compositionengine/DisplayColorProfile.h> @@ -221,6 +222,17 @@ void DisplayDevice::setActiveMode(DisplayModeId modeId, Fps vsyncRate, Fps rende bool DisplayDevice::initiateModeChange(display::DisplayModeRequest&& desiredMode, const hal::VsyncPeriodChangeConstraints& constraints, hal::VsyncPeriodChangeTimeline& outTimeline) { + // TODO(b/255635711): Flow the DisplayModeRequest through the desired/pending/active states. For + // now, `desiredMode` and `mDesiredModeOpt` are one and the same, but the latter is not cleared + // until the next `SF::initiateDisplayModeChanges`. However, the desired mode has been consumed + // at this point, so clear the `force` flag to prevent an endless loop of `initiateModeChange`. + if (FlagManager::getInstance().connected_display()) { + std::scoped_lock lock(mDesiredModeLock); + if (mDesiredModeOpt) { + mDesiredModeOpt->force = false; + } + } + mPendingModeOpt = std::move(desiredMode); mIsModeSetPending = true; @@ -526,8 +538,7 @@ void DisplayDevice::animateOverlay() { } } -auto DisplayDevice::setDesiredMode(display::DisplayModeRequest&& desiredMode, bool force) - -> DesiredModeAction { +auto DisplayDevice::setDesiredMode(display::DisplayModeRequest&& desiredMode) -> DesiredModeAction { ATRACE_CALL(); const auto& desiredModePtr = desiredMode.mode.modePtr; @@ -535,20 +546,26 @@ auto DisplayDevice::setDesiredMode(display::DisplayModeRequest&& desiredMode, bo LOG_ALWAYS_FATAL_IF(getPhysicalId() != desiredModePtr->getPhysicalDisplayId(), "DisplayId mismatch"); - ALOGV("%s(%s)", __func__, to_string(*desiredModePtr).c_str()); + // TODO (b/318533819): Stringize DisplayModeRequest. + ALOGD("%s(%s, force=%s)", __func__, to_string(*desiredModePtr).c_str(), + desiredMode.force ? "true" : "false"); std::scoped_lock lock(mDesiredModeLock); if (mDesiredModeOpt) { // A mode transition was already scheduled, so just override the desired mode. const bool emitEvent = mDesiredModeOpt->emitEvent; + const bool force = mDesiredModeOpt->force; mDesiredModeOpt = std::move(desiredMode); mDesiredModeOpt->emitEvent |= emitEvent; + if (FlagManager::getInstance().connected_display()) { + mDesiredModeOpt->force |= force; + } return DesiredModeAction::None; } // If the desired mode is already active... const auto activeMode = refreshRateSelector().getActiveMode(); - if (!force && activeMode.modePtr->getId() == desiredModePtr->getId()) { + if (!desiredMode.force && activeMode.modePtr->getId() == desiredModePtr->getId()) { if (activeMode == desiredMode.mode) { return DesiredModeAction::None; } diff --git a/services/surfaceflinger/DisplayDevice.h b/services/surfaceflinger/DisplayDevice.h index 97b56a2f19..46534de25e 100644 --- a/services/surfaceflinger/DisplayDevice.h +++ b/services/surfaceflinger/DisplayDevice.h @@ -189,8 +189,7 @@ public: enum class DesiredModeAction { None, InitiateDisplayModeSwitch, InitiateRenderRateSwitch }; - DesiredModeAction setDesiredMode(display::DisplayModeRequest&&, bool force = false) - EXCLUDES(mDesiredModeLock); + DesiredModeAction setDesiredMode(display::DisplayModeRequest&&) EXCLUDES(mDesiredModeLock); using DisplayModeRequestOpt = ftl::Optional<display::DisplayModeRequest>; diff --git a/services/surfaceflinger/DisplayHardware/HWC2.cpp b/services/surfaceflinger/DisplayHardware/HWC2.cpp index 704ece516d..db66f5b549 100644 --- a/services/surfaceflinger/DisplayHardware/HWC2.cpp +++ b/services/surfaceflinger/DisplayHardware/HWC2.cpp @@ -27,6 +27,7 @@ #include "HWC2.h" #include <android/configuration.h> +#include <common/FlagManager.h> #include <ui/Fence.h> #include <ui/FloatRect.h> #include <ui/GraphicBuffer.h> @@ -416,7 +417,19 @@ Error Display::setActiveConfigWithConstraints(hal::HWConfigId configId, VsyncPeriodChangeTimeline* outTimeline) { ALOGV("[%" PRIu64 "] setActiveConfigWithConstraints", mId); - if (isVsyncPeriodSwitchSupported()) { + // FIXME (b/319505580): At least the first config set on an external display must be + // `setActiveConfig`, so skip over the block that calls `setActiveConfigWithConstraints` + // for simplicity. + ui::DisplayConnectionType type = ui::DisplayConnectionType::Internal; + const bool connected_display = FlagManager::getInstance().connected_display(); + if (connected_display) { + if (auto err = getConnectionType(&type); err != Error::NONE) { + return err; + } + } + + if (isVsyncPeriodSwitchSupported() && + (!connected_display || type != ui::DisplayConnectionType::External)) { Hwc2::IComposerClient::VsyncPeriodChangeConstraints hwc2Constraints; hwc2Constraints.desiredTimeNanos = constraints.desiredTimeNanos; hwc2Constraints.seamlessRequired = constraints.seamlessRequired; diff --git a/services/surfaceflinger/SurfaceFlinger.cpp b/services/surfaceflinger/SurfaceFlinger.cpp index c3bfb58786..31d120d185 100644 --- a/services/surfaceflinger/SurfaceFlinger.cpp +++ b/services/surfaceflinger/SurfaceFlinger.cpp @@ -1235,8 +1235,10 @@ status_t SurfaceFlinger::getDisplayStats(const sp<IBinder>& displayToken, return NO_ERROR; } -void SurfaceFlinger::setDesiredMode(display::DisplayModeRequest&& request, bool force) { - const auto displayId = request.mode.modePtr->getPhysicalDisplayId(); +void SurfaceFlinger::setDesiredMode(display::DisplayModeRequest&& desiredMode) { + const auto mode = desiredMode.mode; + const auto displayId = mode.modePtr->getPhysicalDisplayId(); + ATRACE_NAME(ftl::Concat(__func__, ' ', displayId.value).c_str()); const auto display = getDisplayDeviceLocked(displayId); @@ -1245,10 +1247,9 @@ void SurfaceFlinger::setDesiredMode(display::DisplayModeRequest&& request, bool return; } - const auto mode = request.mode; - const bool emitEvent = request.emitEvent; + const bool emitEvent = desiredMode.emitEvent; - switch (display->setDesiredMode(std::move(request), force)) { + switch (display->setDesiredMode(std::move(desiredMode))) { case DisplayDevice::DesiredModeAction::InitiateDisplayModeSwitch: // DisplayDevice::setDesiredMode updated the render rate, so inform Scheduler. mScheduler->setRenderRate(displayId, @@ -1434,7 +1435,8 @@ void SurfaceFlinger::initiateDisplayModeChanges() { to_string(displayModePtrOpt->get()->getVsyncRate()).c_str(), to_string(display->getId()).c_str()); - if (display->getActiveMode() == desiredModeOpt->mode) { + if ((!FlagManager::getInstance().connected_display() || !desiredModeOpt->force) && + display->getActiveMode() == desiredModeOpt->mode) { applyActiveMode(display); continue; } @@ -2449,6 +2451,13 @@ bool SurfaceFlinger::updateLayerSnapshots(VsyncId vsyncId, nsecs_t frameTimeNs, const bool willReleaseBufferOnLatch = layer->willReleaseBufferOnLatch(); auto it = mLegacyLayers.find(layer->id); + if (it == mLegacyLayers.end() && + layer->changes.test(frontend::RequestedLayerState::Changes::Destroyed)) { + // Layer handle was created and immediately destroyed. It was destroyed before it + // was added to the map. + continue; + } + LLOG_ALWAYS_FATAL_WITH_TRACE_IF(it == mLegacyLayers.end(), "Couldnt find layer object for %s", layer->getDebugString().c_str()); @@ -3293,14 +3302,88 @@ std::pair<DisplayModes, DisplayModePtr> SurfaceFlinger::loadDisplayModes( std::vector<HWComposer::HWCDisplayMode> hwcModes; std::optional<hal::HWDisplayId> activeModeHwcId; + const bool isExternalDisplay = FlagManager::getInstance().connected_display() && + getHwComposer().getDisplayConnectionType(displayId) == + ui::DisplayConnectionType::External; + int attempt = 0; constexpr int kMaxAttempts = 3; do { hwcModes = getHwComposer().getModes(displayId, scheduler::RefreshRateSelector::kMinSupportedFrameRate .getPeriodNsecs()); + activeModeHwcId = getHwComposer().getActiveMode(displayId); + if (isExternalDisplay) { + constexpr nsecs_t k59HzVsyncPeriod = 16949153; + constexpr nsecs_t k60HzVsyncPeriod = 16666667; + + // DM sets the initial mode for an external display to 1080p@60, but + // this comes after SF creates its own state (including the + // DisplayDevice). For now, pick the same mode in order to avoid + // inconsistent state and unnecessary mode switching. + // TODO (b/318534874): Let DM decide the initial mode. + // + // Try to find 1920x1080 @ 60 Hz + if (const auto iter = std::find_if(hwcModes.begin(), hwcModes.end(), + [](const auto& mode) { + return mode.width == 1920 && + mode.height == 1080 && + mode.vsyncPeriod == k60HzVsyncPeriod; + }); + iter != hwcModes.end()) { + activeModeHwcId = iter->hwcId; + break; + } + + // Try to find 1920x1080 @ 59-60 Hz + if (const auto iter = std::find_if(hwcModes.begin(), hwcModes.end(), + [](const auto& mode) { + return mode.width == 1920 && + mode.height == 1080 && + mode.vsyncPeriod >= k60HzVsyncPeriod && + mode.vsyncPeriod <= k59HzVsyncPeriod; + }); + iter != hwcModes.end()) { + activeModeHwcId = iter->hwcId; + break; + } + + // The display does not support 1080p@60, and this is the last attempt to pick a display + // mode. Prefer 60 Hz if available, with the closest resolution to 1080p. + if (attempt + 1 == kMaxAttempts) { + std::vector<HWComposer::HWCDisplayMode> hwcModeOpts; + + for (const auto& mode : hwcModes) { + if (mode.width <= 1920 && mode.height <= 1080 && + mode.vsyncPeriod >= k60HzVsyncPeriod && + mode.vsyncPeriod <= k59HzVsyncPeriod) { + hwcModeOpts.push_back(mode); + } + } + + if (const auto iter = std::max_element(hwcModeOpts.begin(), hwcModeOpts.end(), + [](const auto& a, const auto& b) { + const auto aSize = a.width * a.height; + const auto bSize = b.width * b.height; + if (aSize < bSize) + return true; + else if (aSize == bSize) + return a.vsyncPeriod > b.vsyncPeriod; + else + return false; + }); + iter != hwcModeOpts.end()) { + activeModeHwcId = iter->hwcId; + break; + } + + // hwcModeOpts was empty, use hwcModes[0] as the last resort + activeModeHwcId = hwcModes[0].hwcId; + } + } + const auto isActiveMode = [activeModeHwcId](const HWComposer::HWCDisplayMode& mode) { return mode.hwcId == activeModeHwcId; }; @@ -3360,6 +3443,10 @@ std::pair<DisplayModes, DisplayModePtr> SurfaceFlinger::loadDisplayModes( return pair.second->getHwcId() == activeModeHwcId; })->second; + if (isExternalDisplay) { + ALOGI("External display %s initial mode: {%s}", to_string(displayId).c_str(), + to_string(*activeMode).c_str()); + } return {modes, activeMode}; } @@ -3666,6 +3753,27 @@ void SurfaceFlinger::processDisplayAdded(const wp<IBinder>& displayToken, } mDisplays.try_emplace(displayToken, std::move(display)); + + // For an external display, loadDisplayModes already selected the same mode + // as DM, but SF still needs to be updated to match. + // TODO (b/318534874): Let DM decide the initial mode. + if (const auto& physical = state.physical; + mScheduler && physical && FlagManager::getInstance().connected_display()) { + const bool isInternalDisplay = mPhysicalDisplays.get(physical->id) + .transform(&PhysicalDisplay::isInternal) + .value_or(false); + + if (!isInternalDisplay) { + auto activeModePtr = physical->activeMode; + const auto fps = activeModePtr->getPeakFps(); + + setDesiredMode( + {.mode = scheduler::FrameRateMode{fps, + ftl::as_non_null(std::move(activeModePtr))}, + .emitEvent = false, + .force = true}); + } + } } void SurfaceFlinger::processDisplayRemoved(const wp<IBinder>& displayToken) { @@ -8334,7 +8442,7 @@ status_t SurfaceFlinger::applyRefreshRateSelectorPolicy( return INVALID_OPERATION; } - setDesiredMode({std::move(preferredMode), .emitEvent = true}, force); + setDesiredMode({std::move(preferredMode), .emitEvent = true, .force = force}); // Update the frameRateOverride list as the display render rate might have changed if (mScheduler->updateFrameRateOverrides(scheduler::GlobalSignals{}, preferredFps)) { diff --git a/services/surfaceflinger/SurfaceFlinger.h b/services/surfaceflinger/SurfaceFlinger.h index 5846214d97..b23b519b34 100644 --- a/services/surfaceflinger/SurfaceFlinger.h +++ b/services/surfaceflinger/SurfaceFlinger.h @@ -718,7 +718,7 @@ private: // Show hdr sdr ratio overlay bool mHdrSdrRatioOverlay = false; - void setDesiredMode(display::DisplayModeRequest&&, bool force = false) REQUIRES(mStateLock); + void setDesiredMode(display::DisplayModeRequest&&) REQUIRES(mStateLock); status_t setActiveModeFromBackdoor(const sp<display::DisplayToken>&, DisplayModeId, Fps minFps, Fps maxFps); diff --git a/services/surfaceflinger/common/include/common/FlagManager.h b/services/surfaceflinger/common/include/common/FlagManager.h index 2f2895c82e..c01465b04e 100644 --- a/services/surfaceflinger/common/include/common/FlagManager.h +++ b/services/surfaceflinger/common/include/common/FlagManager.h @@ -17,6 +17,7 @@ #pragma once #include <cstdint> +#include <functional> #include <mutex> #include <optional> #include <string> diff --git a/services/surfaceflinger/tests/LayerTransaction_test.cpp b/services/surfaceflinger/tests/LayerTransaction_test.cpp index 03de8d0b6d..ea141f3257 100644 --- a/services/surfaceflinger/tests/LayerTransaction_test.cpp +++ b/services/surfaceflinger/tests/LayerTransaction_test.cpp @@ -213,6 +213,15 @@ TEST_F(LayerTransactionTest, CommitCallbackCalledOnce) { ASSERT_EQ(callCount, 1); } +TEST_F(LayerTransactionTest, AddRemoveLayers) { + for (int i = 0; i < 100; i++) { + sp<SurfaceControl> layer; + ASSERT_NO_FATAL_FAILURE( + layer = createLayer("test", 32, 32, ISurfaceComposerClient::eFXSurfaceBufferState)); + layer.clear(); + } +} + } // namespace android // TODO(b/129481165): remove the #pragma below and fix conversion issues diff --git a/services/surfaceflinger/tests/unittests/DisplayTransactionTestHelpers.h b/services/surfaceflinger/tests/unittests/DisplayTransactionTestHelpers.h index 6671414ba6..99fef7ea24 100644 --- a/services/surfaceflinger/tests/unittests/DisplayTransactionTestHelpers.h +++ b/services/surfaceflinger/tests/unittests/DisplayTransactionTestHelpers.h @@ -447,9 +447,11 @@ struct HwcDisplayVariant { ? IComposerClient::DisplayConnectionType::INTERNAL : IComposerClient::DisplayConnectionType::EXTERNAL; + using ::testing::AtLeast; EXPECT_CALL(*test->mComposer, getDisplayConnectionType(HWC_DISPLAY_ID, _)) - .WillOnce(DoAll(SetArgPointee<1>(CONNECTION_TYPE), - Return(hal::V2_4::Error::NONE))); + .Times(AtLeast(1)) + .WillRepeatedly(DoAll(SetArgPointee<1>(CONNECTION_TYPE), + Return(hal::V2_4::Error::NONE))); } EXPECT_CALL(*test->mComposer, setClientTargetSlotCount(_)) diff --git a/services/surfaceflinger/tests/unittests/SurfaceFlinger_DisplayModeSwitching.cpp b/services/surfaceflinger/tests/unittests/SurfaceFlinger_DisplayModeSwitching.cpp index 8b16a8a480..82b4ad0b4f 100644 --- a/services/surfaceflinger/tests/unittests/SurfaceFlinger_DisplayModeSwitching.cpp +++ b/services/surfaceflinger/tests/unittests/SurfaceFlinger_DisplayModeSwitching.cpp @@ -21,9 +21,13 @@ #include "mock/DisplayHardware/MockDisplayMode.h" #include "mock/MockDisplayModeSpecs.h" +#include <com_android_graphics_surfaceflinger_flags.h> +#include <common/test/FlagUtils.h> #include <ftl/fake_guard.h> #include <scheduler/Fps.h> +using namespace com::android::graphics::surfaceflinger; + namespace android { namespace { @@ -360,6 +364,13 @@ MATCHER_P(ModeSettledTo, modeId, "") { } TEST_F(DisplayModeSwitchingTest, innerXorOuterDisplay) { + SET_FLAG_FOR_TEST(flags::connected_display, true); + + // For the inner display, this is handled by setupHwcHotplugCallExpectations. + EXPECT_CALL(*mComposer, getDisplayConnectionType(kOuterDisplayHwcId, _)) + .WillOnce(DoAll(SetArgPointee<1>(IComposerClient::DisplayConnectionType::INTERNAL), + Return(hal::V2_4::Error::NONE))); + const auto [innerDisplay, outerDisplay] = injectOuterDisplay(); EXPECT_TRUE(innerDisplay->isPoweredOn()); @@ -429,6 +440,12 @@ TEST_F(DisplayModeSwitchingTest, innerXorOuterDisplay) { } TEST_F(DisplayModeSwitchingTest, innerAndOuterDisplay) { + SET_FLAG_FOR_TEST(flags::connected_display, true); + + // For the inner display, this is handled by setupHwcHotplugCallExpectations. + EXPECT_CALL(*mComposer, getDisplayConnectionType(kOuterDisplayHwcId, _)) + .WillOnce(DoAll(SetArgPointee<1>(IComposerClient::DisplayConnectionType::INTERNAL), + Return(hal::V2_4::Error::NONE))); const auto [innerDisplay, outerDisplay] = injectOuterDisplay(); EXPECT_TRUE(innerDisplay->isPoweredOn()); @@ -512,6 +529,13 @@ TEST_F(DisplayModeSwitchingTest, powerOffDuringModeSet) { } TEST_F(DisplayModeSwitchingTest, powerOffDuringConcurrentModeSet) { + SET_FLAG_FOR_TEST(flags::connected_display, true); + + // For the inner display, this is handled by setupHwcHotplugCallExpectations. + EXPECT_CALL(*mComposer, getDisplayConnectionType(kOuterDisplayHwcId, _)) + .WillOnce(DoAll(SetArgPointee<1>(IComposerClient::DisplayConnectionType::INTERNAL), + Return(hal::V2_4::Error::NONE))); + const auto [innerDisplay, outerDisplay] = injectOuterDisplay(); EXPECT_TRUE(innerDisplay->isPoweredOn()); |