From 9e7e909fa1ea8eb6fb2643490cdd72c7d57a9f03 Mon Sep 17 00:00:00 2001 From: Andrew Walbran Date: Fri, 15 Nov 2024 15:57:09 +0000 Subject: Wrap AHardwareBuffer_LockPlanes too. Bug: 371874777 Test: atest libnativewindow_rs-internal_test Change-Id: I1a765c4b0a9455a3d5ed19582a8cfd5593f812fe --- libs/nativewindow/rust/src/lib.rs | 67 +++++++++++++++++++++++++++++++++++++-- 1 file changed, 65 insertions(+), 2 deletions(-) diff --git a/libs/nativewindow/rust/src/lib.rs b/libs/nativewindow/rust/src/lib.rs index 9876362cec..d760285918 100644 --- a/libs/nativewindow/rust/src/lib.rs +++ b/libs/nativewindow/rust/src/lib.rs @@ -30,8 +30,8 @@ use binder::{ StatusCode, }; use ffi::{ - AHardwareBuffer, AHardwareBuffer_Desc, AHardwareBuffer_readFromParcel, - AHardwareBuffer_writeToParcel, ARect, + AHardwareBuffer, AHardwareBuffer_Desc, AHardwareBuffer_Plane, AHardwareBuffer_Planes, + AHardwareBuffer_readFromParcel, AHardwareBuffer_writeToParcel, ARect, }; use std::ffi::c_void; use std::fmt::{self, Debug, Formatter}; @@ -313,6 +313,57 @@ impl HardwareBuffer { }) } + /// Lock a potentially multi-planar hardware buffer for direct CPU access. + /// + /// # Safety + /// + /// - If `fence` is `None`, the caller must ensure that all writes to the buffer have completed + /// before calling this function. + /// - If the buffer has `AHARDWAREBUFFER_FORMAT_BLOB`, multiple threads or process may lock the + /// buffer simultaneously, but the caller must ensure that they don't access it simultaneously + /// and break Rust's aliasing rules, like any other shared memory. + /// - Otherwise if `usage` includes `AHARDWAREBUFFER_USAGE_CPU_WRITE_RARELY` or + /// `AHARDWAREBUFFER_USAGE_CPU_WRITE_OFTEN`, the caller must ensure that no other threads or + /// processes lock the buffer simultaneously for any usage. + /// - Otherwise, the caller must ensure that no other threads lock the buffer for writing + /// simultaneously. + /// - If `rect` is not `None`, the caller must not modify the buffer outside of that rectangle. + pub unsafe fn lock_planes<'a>( + &'a self, + usage: AHardwareBuffer_UsageFlags, + fence: Option, + rect: Option<&ARect>, + ) -> Result>, StatusCode> { + let fence = if let Some(fence) = fence { fence.as_raw_fd() } else { -1 }; + let rect = rect.map(ptr::from_ref).unwrap_or(null()); + let mut planes = AHardwareBuffer_Planes { + planeCount: 0, + planes: [const { AHardwareBuffer_Plane { data: null_mut(), pixelStride: 0, rowStride: 0 } }; + 4], + }; + + // SAFETY: The `AHardwareBuffer` pointer we wrap is always valid, and the various out + // pointers are valid because they come from references. Our caller promises that writes have + // completed and there will be no simultaneous read/write locks. + let status = unsafe { + ffi::AHardwareBuffer_lockPlanes(self.0.as_ptr(), usage.0, fence, rect, &mut planes) + }; + status_result(status)?; + let plane_count = planes.planeCount.try_into().unwrap(); + Ok(planes.planes[..plane_count] + .iter() + .map(|plane| PlaneGuard { + guard: HardwareBufferGuard { + buffer: self, + address: NonNull::new(plane.data) + .expect("AHardwareBuffer_lockAndGetInfo set a null outVirtualAddress"), + }, + pixel_stride: plane.pixelStride, + row_stride: plane.rowStride, + }) + .collect()) + } + /// Locks the hardware buffer for direct CPU access, returning information about the bytes per /// pixel and stride as well. /// @@ -510,6 +561,18 @@ pub struct LockedBufferInfo<'a> { pub stride: u32, } +/// A guard for a single plane of a locked `HardwareBuffer`, with additional information about the +/// stride. +#[derive(Debug)] +pub struct PlaneGuard<'a> { + /// The locked buffer guard. + pub guard: HardwareBufferGuard<'a>, + /// The stride in bytes between the color channel for one pixel to the next pixel. + pub pixel_stride: u32, + /// The stride in bytes between rows in the buffer. + pub row_stride: u32, +} + #[cfg(test)] mod test { use super::*; -- cgit v1.2.3-59-g8ed1b From 7ad5c2b40d648dc204172bcf8c1a303c9d2f6910 Mon Sep 17 00:00:00 2001 From: Emilian Peev Date: Sat, 16 Nov 2024 01:24:38 +0000 Subject: Add HEIC_ULTRAHDR image format Flag: com.android.internal.camera.flags.camera_heif_gainmap Bug: 362608343 Test: atest -c -d cts/tests/camera/src/android/hardware/camera2/cts/ImageReaderTest.java#testHeicUltraHdr Change-Id: Icefca1f6e1676b776a0b083fc7f6f8224cb72762 --- libs/ui/PublicFormat.cpp | 7 +++++++ libs/ui/include/ui/PublicFormat.h | 1 + 2 files changed, 8 insertions(+) diff --git a/libs/ui/PublicFormat.cpp b/libs/ui/PublicFormat.cpp index c9663edd7a..dbc0884bf4 100644 --- a/libs/ui/PublicFormat.cpp +++ b/libs/ui/PublicFormat.cpp @@ -30,6 +30,7 @@ int mapPublicFormatToHalFormat(PublicFormat f) { case PublicFormat::DEPTH_POINT_CLOUD: case PublicFormat::DEPTH_JPEG: case PublicFormat::HEIC: + case PublicFormat::HEIC_ULTRAHDR: case PublicFormat::JPEG_R: return HAL_PIXEL_FORMAT_BLOB; case PublicFormat::DEPTH16: @@ -74,6 +75,9 @@ android_dataspace mapPublicFormatToHalDataspace(PublicFormat f) { case PublicFormat::HEIC: dataspace = Dataspace::HEIF; break; + case PublicFormat::HEIC_ULTRAHDR: + dataspace = Dataspace::HEIF_ULTRAHDR; + break; case PublicFormat::JPEG_R: dataspace = Dataspace::JPEG_R; break; @@ -153,6 +157,9 @@ PublicFormat mapHalFormatDataspaceToPublicFormat(int format, android_dataspace d return PublicFormat::DEPTH_JPEG; } else if (dataSpace == static_cast(Dataspace::JPEG_R)) { return PublicFormat::JPEG_R; + } else if (dataSpace == static_cast( + Dataspace::HEIF_ULTRAHDR)) { + return PublicFormat::HEIC_ULTRAHDR; }else { // Assume otherwise-marked blobs are also JPEG return PublicFormat::JPEG; diff --git a/libs/ui/include/ui/PublicFormat.h b/libs/ui/include/ui/PublicFormat.h index 2248ccab0c..e87931efed 100644 --- a/libs/ui/include/ui/PublicFormat.h +++ b/libs/ui/include/ui/PublicFormat.h @@ -59,6 +59,7 @@ enum class PublicFormat { DEPTH_JPEG = 0x69656963, JPEG_R = 0x1005, HEIC = 0x48454946, + HEIC_ULTRAHDR = 0x1006, }; /* Convert from android.graphics.ImageFormat/PixelFormat enums to graphics.h HAL -- cgit v1.2.3-59-g8ed1b From 28c21c69d6a019dbea8c2087a97f04dd79a9b514 Mon Sep 17 00:00:00 2001 From: Shuangxi Xiang Date: Tue, 12 Nov 2024 02:06:21 +0000 Subject: Complete the trace information LLM to LayerLifecycleManager When analyzing the trace of performance issues, the LLM-related information in the LLM:commitChanges time consumption is unclear to newcomers or non-professionals. It is recommended to complete LLM with LayerLifecycleManager, which can make it clear to the person analyzing the trace and reduce the time loss of searching in the source code. Signed-off-by: Shuangxi Xiang Change-Id: I5ed05d0f5aa4c5e98fbd9b5275f3db39ccb76fed Merged-In: I5ed05d0f5aa4c5e98fbd9b5275f3db39ccb76fed --- services/surfaceflinger/SurfaceFlinger.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/services/surfaceflinger/SurfaceFlinger.cpp b/services/surfaceflinger/SurfaceFlinger.cpp index d4d32aa37a..b85086763b 100644 --- a/services/surfaceflinger/SurfaceFlinger.cpp +++ b/services/surfaceflinger/SurfaceFlinger.cpp @@ -2580,7 +2580,7 @@ bool SurfaceFlinger::updateLayerSnapshots(VsyncId vsyncId, nsecs_t frameTimeNs, } { - ATRACE_NAME("LLM:commitChanges"); + ATRACE_NAME("LayerLifecycleManager:commitChanges"); mLayerLifecycleManager.commitChanges(); } -- cgit v1.2.3-59-g8ed1b From fc4f2817e10f4c145ca51a06de208565aa23c3ad Mon Sep 17 00:00:00 2001 From: Shan Huang Date: Fri, 1 Nov 2024 18:33:43 -0700 Subject: Tweak Kawase2 to use fewer passes and fewer samples. Test: atest librenderengine_bench Bug: 353826438 Flag: EXEMPT behind sysprop flag already Change-Id: Ifcc4e76aa35eb0793734e085ccaeba7d468e4bce --- .../skia/filters/KawaseBlurDualFilter.cpp | 53 ++++++++++++---------- 1 file changed, 30 insertions(+), 23 deletions(-) diff --git a/libs/renderengine/skia/filters/KawaseBlurDualFilter.cpp b/libs/renderengine/skia/filters/KawaseBlurDualFilter.cpp index db0b133a26..da47aae15b 100644 --- a/libs/renderengine/skia/filters/KawaseBlurDualFilter.cpp +++ b/libs/renderengine/skia/filters/KawaseBlurDualFilter.cpp @@ -96,13 +96,17 @@ void KawaseBlurDualFilter::blurInto(const sk_sp& drawSurface, void KawaseBlurDualFilter::blurInto(const sk_sp& drawSurface, sk_sp input, const float inverseScale, const float radius, const float alpha) const { - SkRuntimeShaderBuilder blurBuilder(mBlurEffect); - blurBuilder.child("child") = std::move(input); - blurBuilder.uniform("in_inverseScale") = inverseScale; - blurBuilder.uniform("in_blurOffset") = radius; - blurBuilder.uniform("in_crossFade") = alpha; SkPaint paint; - paint.setShader(blurBuilder.makeShader(nullptr)); + if (radius == 0) { + paint.setShader(std::move(input)); + paint.setAlphaf(alpha); + } else { + SkRuntimeShaderBuilder blurBuilder(mBlurEffect); + blurBuilder.child("child") = std::move(input); + blurBuilder.uniform("in_blurOffset") = radius; + blurBuilder.uniform("in_crossFade") = alpha; + paint.setShader(blurBuilder.makeShader(nullptr)); + } paint.setBlendMode(alpha == 1.0f ? SkBlendMode::kSrc : SkBlendMode::kSrcOver); drawSurface->getCanvas()->drawPaint(paint); } @@ -116,32 +120,35 @@ sk_sp KawaseBlurDualFilter::generate(SkiaGpuContext* context, const uin // Use a variable number of blur passes depending on the radius. The non-integer part of this // calculation is used to mix the final pass into the second-last with an alpha blend. - constexpr int kMaxSurfaces = 4; - const float filterDepth = - std::min(kMaxSurfaces - 1.0f, 1.0f + std::max(0.0f, log2f(radius * kInputScale))); + constexpr int kMaxSurfaces = 3; + const float filterDepth = std::min(kMaxSurfaces - 1.0f, radius * kInputScale / 2.5f); const int filterPasses = std::min(kMaxSurfaces - 1, static_cast(ceil(filterDepth))); - // Render into surfaces downscaled by 1x, 1x, 2x, and 4x from the initial downscale. + // Render into surfaces downscaled by 1x, 2x, and 4x from the initial downscale. sk_sp surfaces[kMaxSurfaces] = {filterPasses >= 0 ? makeSurface(context, blurRect, 1 * kInverseInputScale) : nullptr, - filterPasses >= 1 ? makeSurface(context, blurRect, 1 * kInverseInputScale) : nullptr, - filterPasses >= 2 ? makeSurface(context, blurRect, 2 * kInverseInputScale) : nullptr, - filterPasses >= 3 ? makeSurface(context, blurRect, 4 * kInverseInputScale) : nullptr}; - - // These weights for scaling offsets per-pass are handpicked to look good at 1 <= radius <= 600. - static const float kWeights[7] = {1.0f, 2.0f, 3.5f, 1.0f, 2.0f, 2.0f, 2.0f}; + filterPasses >= 1 ? makeSurface(context, blurRect, 2 * kInverseInputScale) : nullptr, + filterPasses >= 2 ? makeSurface(context, blurRect, 4 * kInverseInputScale) : nullptr}; + + // These weights for scaling offsets per-pass are handpicked to look good at 1 <= radius <= 250. + static const float kWeights[5] = { + 1.0f, // 1st downsampling pass + 1.0f, // 2nd downsampling pass + 1.0f, // 3rd downsampling pass + 0.0f, // 1st upscaling pass. Set to zero to upscale without blurring for performance. + 1.0f, // 2nd upscaling pass + }; // Kawase is an approximation of Gaussian, but behaves differently because it is made up of many // simpler blurs. A transformation is required to approximate the same effect as Gaussian. - float sumSquaredR = powf(kWeights[0] * powf(2.0f, 1), 2.0f); + float sumSquaredR = powf(kWeights[0], 2.0f); for (int i = 0; i < filterPasses; i++) { const float alpha = std::min(1.0f, filterDepth - i); - sumSquaredR += powf(powf(2.0f, i + 1) * alpha * kWeights[1 + i], 2.0f); - sumSquaredR += powf(powf(2.0f, i + 1) * alpha * kWeights[6 - i], 2.0f); + sumSquaredR += powf(powf(2.0f, i) * alpha * kWeights[1 + i] / kInputScale, 2.0f); + sumSquaredR += powf(powf(2.0f, i + 1) * alpha * kWeights[4 - i] / kInputScale, 2.0f); } - // Solve for R = sqrt(sum(r_i^2)). Divide R by hypot(1,1) to find some (x,y) offsets. - const float step = M_SQRT1_2 * - sqrtf(max(0.0f, (powf(radius, 2.0f) - powf(kInverseInputScale, 2.0f)) / sumSquaredR)); + // Solve for R = sqrt(sum(r_i^2)). + const float step = radius * sqrt(1.0f / sumSquaredR); // Start by downscaling and doing the first blur pass. { @@ -162,7 +169,7 @@ sk_sp KawaseBlurDualFilter::generate(SkiaGpuContext* context, const uin } // Finally blur+upscale back to our original size. for (int i = filterPasses - 1; i >= 0; i--) { - blurInto(surfaces[i], surfaces[i + 1]->makeImageSnapshot(), kWeights[6 - i] * step, + blurInto(surfaces[i], surfaces[i + 1]->makeImageSnapshot(), kWeights[4 - i] * step, std::min(1.0f, filterDepth - i)); } return surfaces[0]->makeImageSnapshot(); -- cgit v1.2.3-59-g8ed1b From 081ed30648328dc465b50ff4db4df892aa8b1d4d Mon Sep 17 00:00:00 2001 From: Shuangxi Xiang Date: Mon, 18 Nov 2024 02:37:20 +0000 Subject: Remove the extra parameters in onCommitNotComposited The PhysicalDisplayId parameter of the onCommitNotComposited function is passed by value. PhysicalDisplayId is a structure, but the parameter value is never used inside the onCommitNotComposited function, which will cause a certain amount of memory waste. You can remove this parameter first, or change it to reference passing. (But reference passing here should not be necessary). Signed-off-by: Shuangxi Xiang Change-Id: I996351c8224c286f204975955e5fbbbb52d44e61 --- services/surfaceflinger/Scheduler/ISchedulerCallback.h | 2 +- services/surfaceflinger/Scheduler/Scheduler.cpp | 2 +- services/surfaceflinger/SurfaceFlinger.cpp | 2 +- services/surfaceflinger/SurfaceFlinger.h | 2 +- services/surfaceflinger/tests/unittests/mock/MockSchedulerCallback.h | 4 ++-- 5 files changed, 6 insertions(+), 6 deletions(-) diff --git a/services/surfaceflinger/Scheduler/ISchedulerCallback.h b/services/surfaceflinger/Scheduler/ISchedulerCallback.h index f430526b76..eae8d6d095 100644 --- a/services/surfaceflinger/Scheduler/ISchedulerCallback.h +++ b/services/surfaceflinger/Scheduler/ISchedulerCallback.h @@ -32,7 +32,7 @@ struct ISchedulerCallback { virtual void onChoreographerAttached() = 0; virtual void onExpectedPresentTimePosted(TimePoint, ftl::NonNull, Fps renderRate) = 0; - virtual void onCommitNotComposited(PhysicalDisplayId pacesetterDisplayId) = 0; + virtual void onCommitNotComposited() = 0; virtual void vrrDisplayIdle(bool idle) = 0; protected: diff --git a/services/surfaceflinger/Scheduler/Scheduler.cpp b/services/surfaceflinger/Scheduler/Scheduler.cpp index 5ec7e48332..60f11b81e8 100644 --- a/services/surfaceflinger/Scheduler/Scheduler.cpp +++ b/services/surfaceflinger/Scheduler/Scheduler.cpp @@ -234,7 +234,7 @@ void Scheduler::onFrameSignal(ICompositor& compositor, VsyncId vsyncId, if (FlagManager::getInstance().vrr_config()) { compositor.sendNotifyExpectedPresentHint(pacesetterPtr->displayId); } - mSchedulerCallback.onCommitNotComposited(pacesetterPtr->displayId); + mSchedulerCallback.onCommitNotComposited(); return; } } diff --git a/services/surfaceflinger/SurfaceFlinger.cpp b/services/surfaceflinger/SurfaceFlinger.cpp index d4d32aa37a..605ee72bd1 100644 --- a/services/surfaceflinger/SurfaceFlinger.cpp +++ b/services/surfaceflinger/SurfaceFlinger.cpp @@ -4472,7 +4472,7 @@ void SurfaceFlinger::sendNotifyExpectedPresentHint(PhysicalDisplayId displayId) scheduleNotifyExpectedPresentHint(displayId); } -void SurfaceFlinger::onCommitNotComposited(PhysicalDisplayId pacesetterDisplayId) { +void SurfaceFlinger::onCommitNotComposited() { if (FlagManager::getInstance().commit_not_composited()) { mFrameTimeline->onCommitNotComposited(); } diff --git a/services/surfaceflinger/SurfaceFlinger.h b/services/surfaceflinger/SurfaceFlinger.h index 2369043821..da797f8161 100644 --- a/services/surfaceflinger/SurfaceFlinger.h +++ b/services/surfaceflinger/SurfaceFlinger.h @@ -701,7 +701,7 @@ private: void onChoreographerAttached() override; void onExpectedPresentTimePosted(TimePoint expectedPresentTime, ftl::NonNull, Fps renderRate) override; - void onCommitNotComposited(PhysicalDisplayId pacesetterDisplayId) override + void onCommitNotComposited() override REQUIRES(kMainThreadContext); void vrrDisplayIdle(bool idle) override; diff --git a/services/surfaceflinger/tests/unittests/mock/MockSchedulerCallback.h b/services/surfaceflinger/tests/unittests/mock/MockSchedulerCallback.h index 8f21cdbaa6..0ac4b68933 100644 --- a/services/surfaceflinger/tests/unittests/mock/MockSchedulerCallback.h +++ b/services/surfaceflinger/tests/unittests/mock/MockSchedulerCallback.h @@ -30,7 +30,7 @@ struct SchedulerCallback final : ISchedulerCallback { MOCK_METHOD(void, onChoreographerAttached, (), (override)); MOCK_METHOD(void, onExpectedPresentTimePosted, (TimePoint, ftl::NonNull, Fps), (override)); - MOCK_METHOD(void, onCommitNotComposited, (PhysicalDisplayId), (override)); + MOCK_METHOD(void, onCommitNotComposited, (), (override)); MOCK_METHOD(void, vrrDisplayIdle, (bool), (override)); }; @@ -41,7 +41,7 @@ struct NoOpSchedulerCallback final : ISchedulerCallback { void triggerOnFrameRateOverridesChanged() override {} void onChoreographerAttached() override {} void onExpectedPresentTimePosted(TimePoint, ftl::NonNull, Fps) override {} - void onCommitNotComposited(PhysicalDisplayId) override {} + void onCommitNotComposited() override {} void vrrDisplayIdle(bool) override {} }; -- cgit v1.2.3-59-g8ed1b From 227aef2d74a5a32547c2570a1c85f8f9c8fef4d5 Mon Sep 17 00:00:00 2001 From: Rachel Lee Date: Tue, 26 Nov 2024 11:31:08 -0800 Subject: Add GTE compatibility enum to ANativeWindow. This moves and renames the enum into ANativeWindow where it is accessible in the public NDK. Test: atest SetFrameRateTest Test: atest libsurfaceflinger_unittest Test: atest LayerHistoryIntegrationTest Bug: 380949716 Flag: EXEMPT ndk Change-Id: I5216c3ceb223f7b9a0571be14544e83d7f8859ea --- libs/gui/FrameRateUtils.cpp | 2 +- libs/gui/tests/FrameRateUtilsTest.cpp | 2 +- libs/nativewindow/include/android/native_window.h | 12 +++++++++--- libs/nativewindow/include/system/window.h | 7 +------ services/surfaceflinger/Scheduler/LayerInfo.cpp | 2 +- .../tests/unittests/LayerHistoryIntegrationTest.cpp | 4 ++-- 6 files changed, 15 insertions(+), 14 deletions(-) diff --git a/libs/gui/FrameRateUtils.cpp b/libs/gui/FrameRateUtils.cpp index 01aa7ed43c..5c4879c1bd 100644 --- a/libs/gui/FrameRateUtils.cpp +++ b/libs/gui/FrameRateUtils.cpp @@ -42,7 +42,7 @@ bool ValidateFrameRate(float frameRate, int8_t compatibility, int8_t changeFrame if (compatibility != ANATIVEWINDOW_FRAME_RATE_COMPATIBILITY_DEFAULT && compatibility != ANATIVEWINDOW_FRAME_RATE_COMPATIBILITY_FIXED_SOURCE && - compatibility != ANATIVEWINDOW_FRAME_RATE_GTE && + compatibility != ANATIVEWINDOW_FRAME_RATE_COMPATIBILITY_GTE && (!privileged || (compatibility != ANATIVEWINDOW_FRAME_RATE_EXACT && compatibility != ANATIVEWINDOW_FRAME_RATE_NO_VOTE))) { diff --git a/libs/gui/tests/FrameRateUtilsTest.cpp b/libs/gui/tests/FrameRateUtilsTest.cpp index 04bfb28f65..5d3287d373 100644 --- a/libs/gui/tests/FrameRateUtilsTest.cpp +++ b/libs/gui/tests/FrameRateUtilsTest.cpp @@ -34,7 +34,7 @@ TEST(FrameRateUtilsTest, ValidateFrameRate) { ANATIVEWINDOW_CHANGE_FRAME_RATE_ALWAYS, "")); EXPECT_TRUE(ValidateFrameRate(60.0f, ANATIVEWINDOW_FRAME_RATE_COMPATIBILITY_FIXED_SOURCE, ANATIVEWINDOW_CHANGE_FRAME_RATE_ONLY_IF_SEAMLESS, "")); - EXPECT_TRUE(ValidateFrameRate(60.0f, ANATIVEWINDOW_FRAME_RATE_GTE, + EXPECT_TRUE(ValidateFrameRate(60.0f, ANATIVEWINDOW_FRAME_RATE_COMPATIBILITY_GTE, ANATIVEWINDOW_CHANGE_FRAME_RATE_ONLY_IF_SEAMLESS, "")); // Privileged APIs. diff --git a/libs/nativewindow/include/android/native_window.h b/libs/nativewindow/include/android/native_window.h index 6f816bf614..ed3e8c1a62 100644 --- a/libs/nativewindow/include/android/native_window.h +++ b/libs/nativewindow/include/android/native_window.h @@ -243,8 +243,7 @@ enum ANativeWindow_FrameRateCompatibility { * There are no inherent restrictions on the frame rate of this window. When * the system selects a frame rate other than what the app requested, the * app will be able to run at the system frame rate without requiring pull - * down. This value should be used when displaying game content, UIs, and - * anything that isn't video. + * down. This value should be used when displaying game content. */ ANATIVEWINDOW_FRAME_RATE_COMPATIBILITY_DEFAULT = 0, /** @@ -256,7 +255,14 @@ enum ANativeWindow_FrameRateCompatibility { * stuttering) than it would be if the system had chosen the app's requested * frame rate. This value should be used for video content. */ - ANATIVEWINDOW_FRAME_RATE_COMPATIBILITY_FIXED_SOURCE = 1 + ANATIVEWINDOW_FRAME_RATE_COMPATIBILITY_FIXED_SOURCE = 1, + + /** + * The window requests a frame rate that is greater than or equal to the specified frame rate. + * This value should be used for UIs, animations, scrolling, and anything that is not a game + * or video. + */ + ANATIVEWINDOW_FRAME_RATE_COMPATIBILITY_GTE = 2 }; /** diff --git a/libs/nativewindow/include/system/window.h b/libs/nativewindow/include/system/window.h index 33c303ae71..f669f7781e 100644 --- a/libs/nativewindow/include/system/window.h +++ b/libs/nativewindow/include/system/window.h @@ -1060,12 +1060,7 @@ enum { /** * This surface will vote for the minimum refresh rate. */ - ANATIVEWINDOW_FRAME_RATE_MIN, - - /** - * The surface requests a frame rate that is greater than or equal to `frameRate`. - */ - ANATIVEWINDOW_FRAME_RATE_GTE + ANATIVEWINDOW_FRAME_RATE_MIN }; /* diff --git a/services/surfaceflinger/Scheduler/LayerInfo.cpp b/services/surfaceflinger/Scheduler/LayerInfo.cpp index ff1926e03f..356c30ed5f 100644 --- a/services/surfaceflinger/Scheduler/LayerInfo.cpp +++ b/services/surfaceflinger/Scheduler/LayerInfo.cpp @@ -504,7 +504,7 @@ FrameRateCompatibility LayerInfo::FrameRate::convertCompatibility(int8_t compati return FrameRateCompatibility::Exact; case ANATIVEWINDOW_FRAME_RATE_MIN: return FrameRateCompatibility::Min; - case ANATIVEWINDOW_FRAME_RATE_GTE: + case ANATIVEWINDOW_FRAME_RATE_COMPATIBILITY_GTE: return FrameRateCompatibility::Gte; case ANATIVEWINDOW_FRAME_RATE_NO_VOTE: return FrameRateCompatibility::NoVote; diff --git a/services/surfaceflinger/tests/unittests/LayerHistoryIntegrationTest.cpp b/services/surfaceflinger/tests/unittests/LayerHistoryIntegrationTest.cpp index 767000e6b2..f9bfe97dcb 100644 --- a/services/surfaceflinger/tests/unittests/LayerHistoryIntegrationTest.cpp +++ b/services/surfaceflinger/tests/unittests/LayerHistoryIntegrationTest.cpp @@ -584,7 +584,7 @@ TEST_F(LayerHistoryIntegrationTest, oneLayerExplicitGte_vrr) { auto layer = createLegacyAndFrontedEndLayer(1); showLayer(1); - setFrameRate(1, (33_Hz).getValue(), ANATIVEWINDOW_FRAME_RATE_GTE, + setFrameRate(1, (33_Hz).getValue(), ANATIVEWINDOW_FRAME_RATE_COMPATIBILITY_GTE, ANATIVEWINDOW_CHANGE_FRAME_RATE_ONLY_IF_SEAMLESS); setFrameRateCategory(1, 0); @@ -623,7 +623,7 @@ TEST_F(LayerHistoryIntegrationTest, oneLayerExplicitGte_nonVrr) { auto layer = createLegacyAndFrontedEndLayer(1); showLayer(1); - setFrameRate(1, (33_Hz).getValue(), ANATIVEWINDOW_FRAME_RATE_GTE, + setFrameRate(1, (33_Hz).getValue(), ANATIVEWINDOW_FRAME_RATE_COMPATIBILITY_GTE, ANATIVEWINDOW_CHANGE_FRAME_RATE_ONLY_IF_SEAMLESS); setFrameRateCategory(1, 0); -- cgit v1.2.3-59-g8ed1b From 684eaa791c1ae29e3236106fd2843fdacabf2209 Mon Sep 17 00:00:00 2001 From: Xiang Wang Date: Wed, 23 Oct 2024 17:41:11 -0700 Subject: Add NDK support for thermal headroom callback API Update the thermal headroom thresholds polling API doc on removal of cache update for Android 16. Bug: 360486877 Flag: EXEMPT NDK Test: atest NativeThermalTest NativeThermalUnitTest Change-Id: If5456fdc009b1154101cdae95d8d2943b030fbad --- include/android/thermal.h | 115 +++++++++++++++++++++++++++++++++++++--------- 1 file changed, 94 insertions(+), 21 deletions(-) diff --git a/include/android/thermal.h b/include/android/thermal.h index 7f9d2edfc7..e5d46b5b8a 100644 --- a/include/android/thermal.h +++ b/include/android/thermal.h @@ -140,45 +140,47 @@ void AThermal_releaseManager(AThermalManager* _Nonnull manager) __INTRODUCED_IN( * Available since API level 30. * * @param manager The manager instance to use to query the thermal status. - * Acquired via {@link AThermal_acquireManager}. + * Acquired via {@link AThermal_acquireManager}. * * @return current thermal status, ATHERMAL_STATUS_ERROR on failure. */ AThermalStatus -AThermal_getCurrentThermalStatus(AThermalManager* _Nonnull manager) __INTRODUCED_IN(30); +AThermal_getCurrentThermalStatus(AThermalManager *_Nonnull manager) __INTRODUCED_IN(30); /** - * Register the thermal status listener for thermal status change. + * Register a thermal status listener for thermal status change. * * Available since API level 30. * * @param manager The manager instance to use to register. - * Acquired via {@link AThermal_acquireManager}. - * @param callback The callback function to be called when thermal status updated. + * Acquired via {@link AThermal_acquireManager}. + * @param callback The callback function to be called on system binder thread pool when thermal + * status updated. * @param data The data pointer to be passed when callback is called. * * @return 0 on success * EINVAL if the listener and data pointer were previously added and not removed. - * EPERM if the required permission is not held. - * EPIPE if communication with the system service has failed. + * EPIPE if communication with the system service has failed, the listener will not get + * removed and this call should be retried */ -int AThermal_registerThermalStatusListener(AThermalManager* _Nonnull manager, +int AThermal_registerThermalStatusListener(AThermalManager *_Nonnull manager, AThermal_StatusCallback _Nullable callback, void* _Nullable data) __INTRODUCED_IN(30); /** - * Unregister the thermal status listener previously resgistered. + * Unregister a thermal status listener previously registered. + * + * No subsequent invocations of the callback will occur after this function returns successfully. * * Available since API level 30. * * @param manager The manager instance to use to unregister. - * Acquired via {@link AThermal_acquireManager}. - * @param callback The callback function to be called when thermal status updated. + * Acquired via {@link AThermal_acquireManager}. + * @param callback The callback function that was previously registered. * @param data The data pointer to be passed when callback is called. * * @return 0 on success * EINVAL if the listener and data pointer were not previously added. - * EPERM if the required permission is not held. * EPIPE if communication with the system service has failed. */ int AThermal_unregisterThermalStatusListener(AThermalManager* _Nonnull manager, @@ -254,7 +256,7 @@ typedef struct AThermalHeadroomThreshold AThermalHeadroomThreshold; * The headroom threshold is used to interpret the possible thermal throttling status based on * the headroom prediction. For example, if the headroom threshold for * {@link ATHERMAL_STATUS_LIGHT} is 0.7, and a headroom prediction in 10s returns 0.75 - * (or {@code AThermal_getThermalHeadroom(10)=0.75}), one can expect that in 10 seconds the system + * (or `AThermal_getThermalHeadroom(10)=0.75}`, one can expect that in 10 seconds the system * could be in lightly throttled state if the workload remains the same. The app can consider * taking actions according to the nearest throttling status the difference between the headroom and * the threshold. @@ -262,24 +264,30 @@ typedef struct AThermalHeadroomThreshold AThermalHeadroomThreshold; * For new devices it's guaranteed to have a single sensor, but for older devices with multiple * sensors reporting different threshold values, the minimum threshold is taken to be conservative * on predictions. Thus, when reading real-time headroom, it's not guaranteed that a real-time value - * of 0.75 (or {@code AThermal_getThermalHeadroom(0)}=0.75) exceeding the threshold of 0.7 above + * of 0.75 (or `AThermal_getThermalHeadroom(0)=0.75`) exceeding the threshold of 0.7 above * will always come with lightly throttled state - * (or {@code AThermal_getCurrentThermalStatus()=ATHERMAL_STATUS_LIGHT}) but it can be lower - * (or {@code AThermal_getCurrentThermalStatus()=ATHERMAL_STATUS_NONE}). + * (or `AThermal_getCurrentThermalStatus()=ATHERMAL_STATUS_LIGHT`) but it can be lower + * (or `AThermal_getCurrentThermalStatus()=ATHERMAL_STATUS_NONE`). * While it's always guaranteed that the device won't be throttled heavier than the unmet * threshold's state, so a real-time headroom of 0.75 will never come with * {@link #ATHERMAL_STATUS_MODERATE} but always lower, and 0.65 will never come with * {@link ATHERMAL_STATUS_LIGHT} but {@link #ATHERMAL_STATUS_NONE}. *

- * The returned list of thresholds is cached on first successful query and owned by the thermal - * manager, which will not change between calls to this function. The caller should only need to - * free the manager with {@link AThermal_releaseManager}. + * Starting in Android 16, this polling API may return different results when called depending on + * the device. The new headroom listener API {@link #AThermal_HeadroomCallback} can be used to + * detect headroom thresholds changes. + *

+ * Before API level 36 the returned list of thresholds is cached on first successful query and owned + * by the thermal manager, which will not change between calls to this function. The caller should + * only need to free the manager with {@link AThermal_releaseManager}. + *

* * @param manager The manager instance to use. * Acquired via {@link AThermal_acquireManager}. * @param outThresholds non-null output pointer to null AThermalHeadroomThreshold pointer, which - * will be set to the cached array of thresholds if thermal thresholds are supported - * by the system or device, otherwise nullptr or unmodified. + * will be set to a new array of thresholds if thermal thresholds are supported + * by the system or device, otherwise nullptr or unmodified. The client should + * clean up the thresholds by array-deleting the threshold pointer. * @param size non-null output pointer whose value will be set to the size of the threshold array * or 0 if it's not supported. * @return 0 on success @@ -292,6 +300,71 @@ int AThermal_getThermalHeadroomThresholds(AThermalManager* _Nonnull manager, * _Nullable outThresholds, size_t* _Nonnull size) __INTRODUCED_IN(35); +/** + * Prototype of the function that is called when thermal headroom or thresholds changes. + * It's passed the updated thermal headroom and thresholds as parameters, as well as the + * pointer provided by the client that registered a callback. + * + * @param data The data pointer to be passed when callback is called. + * @param headroom The current non-negative normalized headroom value, also see + * {@link AThermal_getThermalHeadroom}. + * @param forecastHeadroom The forecasted non-negative normalized headroom value, also see + * {@link AThermal_getThermalHeadroom}. + * @param forecastSeconds The seconds used for the forecast by the system. + * @param thresholds The current headroom thresholds. The thresholds pointer will be a constant + * shared across all callbacks registered from the same process, and it will be + * destroyed after all the callbacks are finished. If the client intents to + * persist the values, it should make a copy of it during the callback. + * @param thresholdsCount The count of thresholds. + */ +typedef void (*AThermal_HeadroomCallback)(void *_Nullable data, + float headroom, + float forecastHeadroom, + int forecastSeconds, + const AThermalHeadroomThreshold* _Nullable thresholds, + size_t thresholdsCount); + +/** + * Register a thermal headroom listener for thermal headroom or thresholds change. + * + * Available since API level 36. + * + * @param manager The manager instance to use to register. + * Acquired via {@link AThermal_acquireManager}. + * @param callback The callback function to be called on system binder thread pool when thermal + * headroom or thresholds update. + * @param data The data pointer to be passed when callback is called. + * + * @return 0 on success + * EINVAL if the listener and data pointer were previously added and not removed. + * EPIPE if communication with the system service has failed. + */ +int AThermal_registerThermalHeadroomListener(AThermalManager* _Nonnull manager, + AThermal_HeadroomCallback _Nullable callback, + void* _Nullable data) __INTRODUCED_IN(36); + +/** + * Unregister a thermal headroom listener previously registered. + * + * No subsequent invocations of the callback will occur after this function returns successfully. + * + * Available since API level 36. + * + * @param manager The manager instance to use to unregister. + * Acquired via {@link AThermal_acquireManager}. + * @param callback The callback function that was previously registered. + * @param data The data pointer that was previously registered. + * + * @return 0 on success + * EINVAL if the listener and data pointer were not previously added. + * EPIPE if communication with the system service has failed, the listener will not get + * removed and this call should be retried + */ + +int AThermal_unregisterThermalHeadroomListener(AThermalManager* _Nonnull manager, + AThermal_HeadroomCallback _Nullable callback, + void* _Nullable data) __INTRODUCED_IN(36); + #ifdef __cplusplus } #endif -- cgit v1.2.3-59-g8ed1b From 5f1ffcbc0bfdea0d5dddcfdcb9ff6bec5fe4f75b Mon Sep 17 00:00:00 2001 From: Steven Moreland Date: Mon, 2 Dec 2024 21:15:28 +0000 Subject: Revert "RpcServer: let system allocate vsock ports" This reverts commit 022db680633b483ec5fab4f6b0722981482b3a02. Reason for revert: not in Android 15 release, will be reverted in testing branch only, since it is causing testing issues. Bug: 381486922 Bug: 380785879 Bug: 380934736 Change-Id: I9a1f0d9b18fc01898b61e338300fad3b4bdbe1a6 Merged-In: I2eec99a8e8f894546e025455cc0c5eb58962395f --- libs/binder/RpcServer.cpp | 19 ++--------------- libs/binder/include/binder/RpcServer.h | 7 ++---- libs/binder/tests/BinderRpcTestServerConfig.aidl | 1 + libs/binder/tests/binderRpcTest.cpp | 27 +++++++++++++++--------- libs/binder/tests/binderRpcTestService.cpp | 4 ++-- 5 files changed, 24 insertions(+), 34 deletions(-) diff --git a/libs/binder/RpcServer.cpp b/libs/binder/RpcServer.cpp index b8742af1f9..d9e926a9d5 100644 --- a/libs/binder/RpcServer.cpp +++ b/libs/binder/RpcServer.cpp @@ -71,23 +71,8 @@ status_t RpcServer::setupUnixDomainServer(const char* path) { return setupSocketServer(UnixSocketAddress(path)); } -status_t RpcServer::setupVsockServer(unsigned bindCid, unsigned port, unsigned* assignedPort) { - auto status = setupSocketServer(VsockSocketAddress(bindCid, port)); - if (status != OK) return status; - - if (assignedPort == nullptr) return OK; - sockaddr_vm addr; - socklen_t len = sizeof(addr); - if (0 != getsockname(mServer.fd.get(), reinterpret_cast(&addr), &len)) { - status = -errno; - ALOGE("setupVsockServer: Failed to getsockname: %s", strerror(-status)); - return status; - } - - LOG_ALWAYS_FATAL_IF(len != sizeof(addr), "Wrong socket type: len %zu vs len %zu", - static_cast(len), sizeof(addr)); - *assignedPort = addr.svm_port; - return OK; +status_t RpcServer::setupVsockServer(unsigned int bindCid, unsigned int port) { + return setupSocketServer(VsockSocketAddress(bindCid, port)); } status_t RpcServer::setupInetServer(const char* address, unsigned int port, diff --git a/libs/binder/include/binder/RpcServer.h b/libs/binder/include/binder/RpcServer.h index c241d31623..abea0fb40c 100644 --- a/libs/binder/include/binder/RpcServer.h +++ b/libs/binder/include/binder/RpcServer.h @@ -85,12 +85,9 @@ public: /** * Creates an RPC server binding to the given CID at the given port. - * - * Set |port| to VMADDR_PORT_ANY to pick an ephemeral port. In this case, |assignedPort| - * will be set to the picked port number, if it is not null. */ - [[nodiscard]] LIBBINDER_EXPORTED status_t setupVsockServer(unsigned bindCid, unsigned port, - unsigned* assignedPort = nullptr); + [[nodiscard]] LIBBINDER_EXPORTED status_t setupVsockServer(unsigned int bindCid, + unsigned int port); /** * Creates an RPC server at the current port using IPv4. diff --git a/libs/binder/tests/BinderRpcTestServerConfig.aidl b/libs/binder/tests/BinderRpcTestServerConfig.aidl index 96550bca6b..b2e0ef21c7 100644 --- a/libs/binder/tests/BinderRpcTestServerConfig.aidl +++ b/libs/binder/tests/BinderRpcTestServerConfig.aidl @@ -20,6 +20,7 @@ parcelable BinderRpcTestServerConfig { int socketType; int rpcSecurity; int serverVersion; + int vsockPort; int socketFd; // Inherited from the parent process. @utf8InCpp String addr; } diff --git a/libs/binder/tests/binderRpcTest.cpp b/libs/binder/tests/binderRpcTest.cpp index d88048b96d..cec0dbf54d 100644 --- a/libs/binder/tests/binderRpcTest.cpp +++ b/libs/binder/tests/binderRpcTest.cpp @@ -141,6 +141,11 @@ static std::string allocateSocketAddress() { return ret; }; +static unsigned int allocateVsockPort() { + static unsigned int vsockPort = 34567; + return vsockPort++; +} + static unique_fd initUnixSocket(std::string addr) { auto socket_addr = UnixSocketAddress(addr.c_str()); unique_fd fd(TEMP_FAILURE_RETRY(socket(socket_addr.addr()->sa_family, SOCK_STREAM, AF_UNIX))); @@ -295,6 +300,7 @@ std::unique_ptr BinderRpc::createRpcTestSocketServerProcessEtc( serverConfig.socketType = static_cast(socketType); serverConfig.rpcSecurity = static_cast(rpcSecurity); serverConfig.serverVersion = serverVersion; + serverConfig.vsockPort = allocateVsockPort(); serverConfig.addr = addr; serverConfig.socketFd = socketFd.get(); for (auto mode : options.serverSupportedFileDescriptorTransportModes) { @@ -373,7 +379,7 @@ std::unique_ptr BinderRpc::createRpcTestSocketServerProcessEtc( unique_fd(dup(bootstrapClientFd.get()))); break; case SocketType::VSOCK: - status = session->setupVsockClient(VMADDR_CID_LOCAL, serverInfo.port); + status = session->setupVsockClient(VMADDR_CID_LOCAL, serverConfig.vsockPort); break; case SocketType::INET: status = session->setupInetClient("127.0.0.1", serverInfo.port); @@ -1146,6 +1152,8 @@ INSTANTIATE_TEST_SUITE_P(Trusty, BinderRpc, ::testing::ValuesIn(getTrustyBinderR #else // BINDER_RPC_TO_TRUSTY_TEST bool testSupportVsockLoopback() { // We don't need to enable TLS to know if vsock is supported. + unsigned int vsockPort = allocateVsockPort(); + unique_fd serverFd( TEMP_FAILURE_RETRY(socket(AF_VSOCK, SOCK_STREAM | SOCK_CLOEXEC | SOCK_NONBLOCK, 0))); @@ -1157,12 +1165,12 @@ bool testSupportVsockLoopback() { sockaddr_vm serverAddr{ .svm_family = AF_VSOCK, - .svm_port = VMADDR_PORT_ANY, + .svm_port = vsockPort, .svm_cid = VMADDR_CID_ANY, }; int ret = TEMP_FAILURE_RETRY( bind(serverFd.get(), reinterpret_cast(&serverAddr), sizeof(serverAddr))); - LOG_ALWAYS_FATAL_IF(0 != ret, "Could not bind socket to port VMADDR_PORT_ANY: %s", + LOG_ALWAYS_FATAL_IF(0 != ret, "Could not bind socket to port %u: %s", vsockPort, strerror(errno)); socklen_t len = sizeof(serverAddr); @@ -1172,7 +1180,7 @@ bool testSupportVsockLoopback() { "getsockname didn't read the full addr struct"); ret = TEMP_FAILURE_RETRY(listen(serverFd.get(), 1 /*backlog*/)); - LOG_ALWAYS_FATAL_IF(0 != ret, "Could not listen socket on port %u: %s", serverAddr.svm_port, + LOG_ALWAYS_FATAL_IF(0 != ret, "Could not listen socket on port %u: %s", vsockPort, strerror(errno)); // Try to connect to the server using the VMADDR_CID_LOCAL cid @@ -1181,13 +1189,13 @@ bool testSupportVsockLoopback() { // and they return ETIMEDOUT after that. unique_fd connectFd( TEMP_FAILURE_RETRY(socket(AF_VSOCK, SOCK_STREAM | SOCK_CLOEXEC | SOCK_NONBLOCK, 0))); - LOG_ALWAYS_FATAL_IF(!connectFd.ok(), "Could not create socket for port %u: %s", - serverAddr.svm_port, strerror(errno)); + LOG_ALWAYS_FATAL_IF(!connectFd.ok(), "Could not create socket for port %u: %s", vsockPort, + strerror(errno)); bool success = false; sockaddr_vm connectAddr{ .svm_family = AF_VSOCK, - .svm_port = serverAddr.svm_port, + .svm_port = vsockPort, .svm_cid = VMADDR_CID_LOCAL, }; ret = TEMP_FAILURE_RETRY(connect(connectFd.get(), reinterpret_cast(&connectAddr), @@ -1536,9 +1544,8 @@ public: }; } break; case SocketType::VSOCK: { - unsigned port; - auto status = - rpcServer->setupVsockServer(VMADDR_CID_LOCAL, VMADDR_PORT_ANY, &port); + auto port = allocateVsockPort(); + auto status = rpcServer->setupVsockServer(VMADDR_CID_LOCAL, port); if (status != OK) { return AssertionFailure() << "setupVsockServer: " << statusToString(status); } diff --git a/libs/binder/tests/binderRpcTestService.cpp b/libs/binder/tests/binderRpcTestService.cpp index aef946481f..28125f169c 100644 --- a/libs/binder/tests/binderRpcTestService.cpp +++ b/libs/binder/tests/binderRpcTestService.cpp @@ -143,8 +143,8 @@ int main(int argc, char* argv[]) { break; case SocketType::VSOCK: LOG_ALWAYS_FATAL_IF(OK != - server->setupVsockServer(VMADDR_CID_LOCAL, VMADDR_PORT_ANY, - &outPort), + server->setupVsockServer(VMADDR_CID_LOCAL, + serverConfig.vsockPort), "Need `sudo modprobe vsock_loopback`?"); break; case SocketType::INET: { -- cgit v1.2.3-59-g8ed1b From 67bbf97106b0831a7f3656607ebe5a3e5fe03b0b Mon Sep 17 00:00:00 2001 From: Steven Moreland Date: Thu, 28 Nov 2024 01:32:33 +0000 Subject: binder: test FrameworksCoreTests_all_binder Not sure why we didn't test these core changes for changes here. Bug: 348072409 Change-Id: I02a14eb166a4337c411704a2bd4de089ec147ff0 Test: TH --- libs/binder/TEST_MAPPING | 3 +++ 1 file changed, 3 insertions(+) diff --git a/libs/binder/TEST_MAPPING b/libs/binder/TEST_MAPPING index 99a9c911bf..9e5e79f89b 100644 --- a/libs/binder/TEST_MAPPING +++ b/libs/binder/TEST_MAPPING @@ -69,6 +69,9 @@ { "name": "CtsOsTestCases_ParcelAndBinderTests" }, + { + "name": "FrameworksCoreTests_all_binder" + }, { "name": "libbinder_rs-internal_test" }, -- cgit v1.2.3-59-g8ed1b From 7f21fa319eee46ff12f305106ae3bb59c7560332 Mon Sep 17 00:00:00 2001 From: Arpit Singh Date: Tue, 26 Nov 2024 15:44:26 +0000 Subject: Refactor to replace FloatPoint by vec2 Using FloatPoint requires unnecessory conversion between vec2 and FloatPoint, which is inefficient. This CL replaces FloatPoint by vec2 everywhere. Test: atest inputflinger_tests Bug: 245989146 Flag: EXEMPT refactor Change-Id: I61e64e43f6bbe643eb22e758a2934294037d8a5b --- services/inputflinger/PointerChoreographer.cpp | 47 ++++++------ services/inputflinger/PointerChoreographer.h | 12 +-- .../include/PointerChoreographerPolicyInterface.h | 2 +- .../include/PointerControllerInterface.h | 18 +---- .../inputflinger/tests/FakePointerController.cpp | 12 +-- .../inputflinger/tests/FakePointerController.h | 4 +- services/inputflinger/tests/InterfaceMocks.h | 2 +- .../tests/PointerChoreographer_test.cpp | 87 ++++++++++------------ 8 files changed, 81 insertions(+), 103 deletions(-) diff --git a/services/inputflinger/PointerChoreographer.cpp b/services/inputflinger/PointerChoreographer.cpp index 8eb6bdd02f..811692f1bf 100644 --- a/services/inputflinger/PointerChoreographer.cpp +++ b/services/inputflinger/PointerChoreographer.cpp @@ -64,9 +64,8 @@ bool isMouseOrTouchpad(uint32_t sources) { !isFromSource(sources, AINPUT_SOURCE_STYLUS)); } -inline void notifyPointerDisplayChange( - std::optional> change, - PointerChoreographerPolicyInterface& policy) { +inline void notifyPointerDisplayChange(std::optional> change, + PointerChoreographerPolicyInterface& policy) { if (!change) { return; } @@ -245,9 +244,9 @@ NotifyMotionArgs PointerChoreographer::processMouseEventLocked(const NotifyMotio if (MotionEvent::isValidCursorPosition(args.xCursorPosition, args.yCursorPosition)) { // This is an absolute mouse device that knows about the location of the cursor on the // display, so set the cursor position to the specified location. - const auto [x, y] = pc.getPosition(); - const float deltaX = args.xCursorPosition - x; - const float deltaY = args.yCursorPosition - y; + const auto position = pc.getPosition(); + const float deltaX = args.xCursorPosition - position.x; + const float deltaY = args.yCursorPosition - position.y; newArgs.pointerCoords[0].setAxisValue(AMOTION_EVENT_AXIS_RELATIVE_X, deltaX); newArgs.pointerCoords[0].setAxisValue(AMOTION_EVENT_AXIS_RELATIVE_Y, deltaY); pc.setPosition(args.xCursorPosition, args.yCursorPosition); @@ -273,15 +272,15 @@ NotifyMotionArgs PointerChoreographer::processTouchpadEventLocked(const NotifyMo processPointerDeviceMotionEventLocked(/*byref*/ newArgs, /*byref*/ pc); } else { // This is a trackpad gesture with fake finger(s) that should not move the mouse pointer. - const auto [x, y] = pc.getPosition(); + const auto position = pc.getPosition(); for (uint32_t i = 0; i < newArgs.getPointerCount(); i++) { newArgs.pointerCoords[i].setAxisValue(AMOTION_EVENT_AXIS_X, - args.pointerCoords[i].getX() + x); + args.pointerCoords[i].getX() + position.x); newArgs.pointerCoords[i].setAxisValue(AMOTION_EVENT_AXIS_Y, - args.pointerCoords[i].getY() + y); + args.pointerCoords[i].getY() + position.y); } - newArgs.xCursorPosition = x; - newArgs.yCursorPosition = y; + newArgs.xCursorPosition = position.x; + newArgs.yCursorPosition = position.y; } // Note displayId may have changed if the cursor moved to a different display @@ -296,7 +295,7 @@ void PointerChoreographer::processPointerDeviceMotionEventLocked(NotifyMotionArg const float deltaX = newArgs.pointerCoords[0].getAxisValue(AMOTION_EVENT_AXIS_RELATIVE_X); const float deltaY = newArgs.pointerCoords[0].getAxisValue(AMOTION_EVENT_AXIS_RELATIVE_Y); - FloatPoint unconsumedDelta = pc.move(deltaX, deltaY); + vec2 unconsumedDelta = pc.move(deltaX, deltaY); if (com::android::input::flags::connected_displays_cursor() && (std::abs(unconsumedDelta.x) > 0 || std::abs(unconsumedDelta.y) > 0)) { handleUnconsumedDeltaLocked(pc, unconsumedDelta); @@ -304,25 +303,23 @@ void PointerChoreographer::processPointerDeviceMotionEventLocked(NotifyMotionArg newArgs.displayId = pc.getDisplayId(); } - const auto [x, y] = pc.getPosition(); - newArgs.pointerCoords[0].setAxisValue(AMOTION_EVENT_AXIS_X, x); - newArgs.pointerCoords[0].setAxisValue(AMOTION_EVENT_AXIS_Y, y); - newArgs.xCursorPosition = x; - newArgs.yCursorPosition = y; + const auto position = pc.getPosition(); + newArgs.pointerCoords[0].setAxisValue(AMOTION_EVENT_AXIS_X, position.x); + newArgs.pointerCoords[0].setAxisValue(AMOTION_EVENT_AXIS_Y, position.y); + newArgs.xCursorPosition = position.x; + newArgs.yCursorPosition = position.y; } void PointerChoreographer::handleUnconsumedDeltaLocked(PointerControllerInterface& pc, - const FloatPoint& unconsumedDelta) { + const vec2& unconsumedDelta) { // Display topology is in rotated coordinate space and Pointer controller returns and expects // values in the un-rotated coordinate space. So we need to transform delta and cursor position // back to the rotated coordinate space to lookup adjacent display in the display topology. const auto& sourceDisplayTransform = pc.getDisplayTransform(); const vec2 rotatedUnconsumedDelta = - transformWithoutTranslation(sourceDisplayTransform, - {unconsumedDelta.x, unconsumedDelta.y}); - const FloatPoint cursorPosition = pc.getPosition(); - const vec2 rotatedCursorPosition = - sourceDisplayTransform.transform(cursorPosition.x, cursorPosition.y); + transformWithoutTranslation(sourceDisplayTransform, unconsumedDelta); + const vec2 cursorPosition = pc.getPosition(); + const vec2 rotatedCursorPosition = sourceDisplayTransform.transform(cursorPosition); // To find out the boundary that cursor is crossing we are checking delta in x and y direction // respectively. This prioritizes x direction over y. @@ -769,7 +766,7 @@ PointerChoreographer::PointerDisplayChange PointerChoreographer::updatePointerCo PointerChoreographer::PointerDisplayChange PointerChoreographer::calculatePointerDisplayChangeToNotify() { ui::LogicalDisplayId displayIdToNotify = ui::LogicalDisplayId::INVALID; - FloatPoint cursorPosition = {0, 0}; + vec2 cursorPosition = {0, 0}; if (const auto it = mMousePointersByDisplay.find(mDefaultMouseDisplayId); it != mMousePointersByDisplay.end()) { const auto& pointerController = it->second; @@ -840,7 +837,7 @@ std::optional PointerChoreographer::getViewportForPointerDevice return std::nullopt; } -FloatPoint PointerChoreographer::getMouseCursorPosition(ui::LogicalDisplayId displayId) { +vec2 PointerChoreographer::getMouseCursorPosition(ui::LogicalDisplayId displayId) { std::scoped_lock _l(getLock()); const ui::LogicalDisplayId resolvedDisplayId = getTargetMouseDisplayLocked(displayId); if (auto it = mMousePointersByDisplay.find(resolvedDisplayId); diff --git a/services/inputflinger/PointerChoreographer.h b/services/inputflinger/PointerChoreographer.h index 4ca7323bfc..939529f774 100644 --- a/services/inputflinger/PointerChoreographer.h +++ b/services/inputflinger/PointerChoreographer.h @@ -59,7 +59,7 @@ public: virtual void setDisplayViewports(const std::vector& viewports) = 0; virtual std::optional getViewportForPointerDevice( ui::LogicalDisplayId associatedDisplayId = ui::LogicalDisplayId::INVALID) = 0; - virtual FloatPoint getMouseCursorPosition(ui::LogicalDisplayId displayId) = 0; + virtual vec2 getMouseCursorPosition(ui::LogicalDisplayId displayId) = 0; virtual void setShowTouchesEnabled(bool enabled) = 0; virtual void setStylusPointerIconEnabled(bool enabled) = 0; /** @@ -96,7 +96,7 @@ public: void setDisplayViewports(const std::vector& viewports) override; std::optional getViewportForPointerDevice( ui::LogicalDisplayId associatedDisplayId) override; - FloatPoint getMouseCursorPosition(ui::LogicalDisplayId displayId) override; + vec2 getMouseCursorPosition(ui::LogicalDisplayId displayId) override; void setShowTouchesEnabled(bool enabled) override; void setStylusPointerIconEnabled(bool enabled) override; bool setPointerIcon(std::variant, PointerIconStyle> icon, @@ -134,8 +134,8 @@ public: void dump(std::string& dump) override; private: - using PointerDisplayChange = std::optional< - std::tuple>; + using PointerDisplayChange = + std::optional>; // PointerChoreographer's DisplayInfoListener can outlive the PointerChoreographer because when // the listener is registered and called from display thread, a strong pointer to the listener @@ -171,8 +171,8 @@ private: const std::unordered_set& privacySensitiveDisplays) REQUIRES(getLock()); - void handleUnconsumedDeltaLocked(PointerControllerInterface& pc, - const FloatPoint& unconsumedDelta) REQUIRES(getLock()); + void handleUnconsumedDeltaLocked(PointerControllerInterface& pc, const vec2& unconsumedDelta) + REQUIRES(getLock()); void populateFakeDisplayTopologyLocked(const std::vector& displayInfos) REQUIRES(getLock()); diff --git a/services/inputflinger/include/PointerChoreographerPolicyInterface.h b/services/inputflinger/include/PointerChoreographerPolicyInterface.h index e1f8fdaaa0..36614b2c95 100644 --- a/services/inputflinger/include/PointerChoreographerPolicyInterface.h +++ b/services/inputflinger/include/PointerChoreographerPolicyInterface.h @@ -54,7 +54,7 @@ public: * @param position The new position of the mouse cursor on the logical display */ virtual void notifyPointerDisplayIdChanged(ui::LogicalDisplayId displayId, - const FloatPoint& position) = 0; + const vec2& position) = 0; /* Returns true if any InputConnection is currently active. */ virtual bool isInputMethodConnectionActive() = 0; diff --git a/services/inputflinger/include/PointerControllerInterface.h b/services/inputflinger/include/PointerControllerInterface.h index abca209cec..bd464901b6 100644 --- a/services/inputflinger/include/PointerControllerInterface.h +++ b/services/inputflinger/include/PointerControllerInterface.h @@ -24,20 +24,6 @@ namespace android { struct SpriteIcon; -struct FloatPoint { - float x; - float y; - - inline FloatPoint(float x, float y) : x(x), y(y) {} - - inline explicit FloatPoint(vec2 p) : x(p.x), y(p.y) {} - - template - operator std::tuple() { - return {x, y}; - } -}; - /** * Interface for tracking a mouse / touch pad pointer and touch pad spots. * @@ -77,13 +63,13 @@ public: * * Return value may be used to move pointer to corresponding adjacent display, if it exists in * the display-topology */ - [[nodiscard]] virtual FloatPoint move(float deltaX, float deltaY) = 0; + [[nodiscard]] virtual vec2 move(float deltaX, float deltaY) = 0; /* Sets the absolute location of the pointer. */ virtual void setPosition(float x, float y) = 0; /* Gets the absolute location of the pointer. */ - virtual FloatPoint getPosition() const = 0; + virtual vec2 getPosition() const = 0; enum class Transition { // Fade/unfade immediately. diff --git a/services/inputflinger/tests/FakePointerController.cpp b/services/inputflinger/tests/FakePointerController.cpp index f53e63ba25..f033e57ddd 100644 --- a/services/inputflinger/tests/FakePointerController.cpp +++ b/services/inputflinger/tests/FakePointerController.cpp @@ -43,7 +43,7 @@ void FakePointerController::setPosition(float x, float y) { mY = y; } -FloatPoint FakePointerController::getPosition() const { +vec2 FakePointerController::getPosition() const { if (!mEnabled) { return {0, 0}; } @@ -96,9 +96,9 @@ void FakePointerController::assertViewportNotSet() { } void FakePointerController::assertPosition(float x, float y) { - const auto [actualX, actualY] = getPosition(); - ASSERT_NEAR(x, actualX, 1); - ASSERT_NEAR(y, actualY, 1); + const auto actual = getPosition(); + ASSERT_NEAR(x, actual.x, 1); + ASSERT_NEAR(y, actual.y, 1); } void FakePointerController::assertSpotCount(ui::LogicalDisplayId displayId, int32_t count) { @@ -148,13 +148,13 @@ bool FakePointerController::isPointerShown() { return mIsPointerShown; } -FloatPoint FakePointerController::move(float deltaX, float deltaY) { +vec2 FakePointerController::move(float deltaX, float deltaY) { if (!mEnabled) return {0, 0}; mX += deltaX; mY += deltaY; - const FloatPoint position(mX, mY); + const vec2 position(mX, mY); if (mX < mMinX) mX = mMinX; if (mX > mMaxX) mX = mMaxX; diff --git a/services/inputflinger/tests/FakePointerController.h b/services/inputflinger/tests/FakePointerController.h index 0ee3123d35..c526bb8c59 100644 --- a/services/inputflinger/tests/FakePointerController.h +++ b/services/inputflinger/tests/FakePointerController.h @@ -40,7 +40,7 @@ public: const std::map>& getSpots(); void setPosition(float x, float y) override; - FloatPoint getPosition() const override; + vec2 getPosition() const override; ui::LogicalDisplayId getDisplayId() const override; void setDisplayViewport(const DisplayViewport& viewport) override; void updatePointerIcon(PointerIconStyle iconId) override; @@ -66,7 +66,7 @@ public: private: std::string dump() override { return ""; } - FloatPoint move(float deltaX, float deltaY) override; + vec2 move(float deltaX, float deltaY) override; void unfade(Transition) override; void setPresentation(Presentation) override {} void setSpots(const PointerCoords*, const uint32_t*, BitSet32 spotIdBits, diff --git a/services/inputflinger/tests/InterfaceMocks.h b/services/inputflinger/tests/InterfaceMocks.h index 6f7c2e5271..ac616d0296 100644 --- a/services/inputflinger/tests/InterfaceMocks.h +++ b/services/inputflinger/tests/InterfaceMocks.h @@ -188,7 +188,7 @@ public: MOCK_METHOD(std::shared_ptr, createPointerController, (PointerControllerInterface::ControllerType), (override)); MOCK_METHOD(void, notifyPointerDisplayIdChanged, - (ui::LogicalDisplayId displayId, const FloatPoint& position), (override)); + (ui::LogicalDisplayId displayId, const vec2& position), (override)); MOCK_METHOD(bool, isInputMethodConnectionActive, (), (override)); MOCK_METHOD(void, notifyMouseCursorFadedOnTyping, (), (override)); }; diff --git a/services/inputflinger/tests/PointerChoreographer_test.cpp b/services/inputflinger/tests/PointerChoreographer_test.cpp index 453c156665..27da3d33f9 100644 --- a/services/inputflinger/tests/PointerChoreographer_test.cpp +++ b/services/inputflinger/tests/PointerChoreographer_test.cpp @@ -135,7 +135,7 @@ protected: }); ON_CALL(mMockPolicy, notifyPointerDisplayIdChanged) - .WillByDefault([this](ui::LogicalDisplayId displayId, const FloatPoint& position) { + .WillByDefault([this](ui::LogicalDisplayId displayId, const vec2& position) { mPointerDisplayIdNotified = displayId; }); } @@ -2604,9 +2604,8 @@ TEST_P(PointerVisibilityAndTouchpadTapStateOnKeyPressTestFixture, TestMetaKeyCom using PointerChoreographerDisplayTopologyTestFixtureParam = std::tuple; + vec2 /*source position*/, vec2 /*hover move X/Y*/, + ui::LogicalDisplayId /*destination display*/, vec2 /*destination position*/>; class PointerChoreographerDisplayTopologyTestFixture : public PointerChoreographerTest, @@ -2708,67 +2707,63 @@ INSTANTIATE_TEST_SUITE_P( // Note: Upon viewport transition cursor will be positioned at the boundary of the // destination, as we drop any unconsumed delta. std::make_tuple("UnchangedDisplay", AINPUT_SOURCE_MOUSE, ControllerType::MOUSE, - ToolType::MOUSE, FloatPoint(50, 50) /* initial x/y */, - FloatPoint(25, 25) /* delta x/y */, + ToolType::MOUSE, vec2(50, 50) /* initial x/y */, + vec2(25, 25) /* delta x/y */, PointerChoreographerDisplayTopologyTestFixture::DISPLAY_CENTER_ID, - FloatPoint(75, 75) /* destination x/y */), - std::make_tuple( - "TransitionToRightDisplay", AINPUT_SOURCE_MOUSE, ControllerType::MOUSE, - ToolType::MOUSE, FloatPoint(50, 50) /* initial x/y */, - FloatPoint(100, 25) /* delta x/y */, - PointerChoreographerDisplayTopologyTestFixture::DISPLAY_RIGHT_ID, - FloatPoint(0, 50 + 25 - 10) /* Left edge: (0, source + delta - offset) */), + vec2(75, 75) /* destination x/y */), + std::make_tuple("TransitionToRightDisplay", AINPUT_SOURCE_MOUSE, + ControllerType::MOUSE, ToolType::MOUSE, + vec2(50, 50) /* initial x/y */, vec2(100, 25) /* delta x/y */, + PointerChoreographerDisplayTopologyTestFixture::DISPLAY_RIGHT_ID, + vec2(0, + 50 + 25 - 10) /* Left edge: (0, source + delta - offset) */), std::make_tuple( "TransitionToLeftDisplay", AINPUT_SOURCE_MOUSE, ControllerType::MOUSE, - ToolType::MOUSE, FloatPoint(50, 50) /* initial x/y */, - FloatPoint(-100, 25) /* delta x/y */, + ToolType::MOUSE, vec2(50, 50) /* initial x/y */, + vec2(-100, 25) /* delta x/y */, PointerChoreographerDisplayTopologyTestFixture::DISPLAY_LEFT_ID, - FloatPoint(90, - 50 + 25 - 10) /* Right edge: (width, source + delta - offset*/), - std::make_tuple( - "TransitionToTopDisplay", AINPUT_SOURCE_MOUSE | AINPUT_SOURCE_TOUCHPAD, - ControllerType::MOUSE, ToolType::FINGER, - FloatPoint(50, 50) /* initial x/y */, FloatPoint(25, -100) /* delta x/y */, - PointerChoreographerDisplayTopologyTestFixture::DISPLAY_TOP_ID, - FloatPoint(50 + 25 - 10, - 90) /* Bottom edge: (source + delta - offset, height) */), - std::make_tuple( - "TransitionToBottomDisplay", AINPUT_SOURCE_MOUSE | AINPUT_SOURCE_TOUCHPAD, - ControllerType::MOUSE, ToolType::FINGER, - FloatPoint(50, 50) /* initial x/y */, FloatPoint(25, 100) /* delta x/y */, - PointerChoreographerDisplayTopologyTestFixture::DISPLAY_BOTTOM_ID, - FloatPoint(50 + 25 - 10, 0) /* Top edge: (source + delta - offset, 0) */), + vec2(90, 50 + 25 - 10) /* Right edge: (width, source + delta - offset*/), + std::make_tuple("TransitionToTopDisplay", + AINPUT_SOURCE_MOUSE | AINPUT_SOURCE_TOUCHPAD, ControllerType::MOUSE, + ToolType::FINGER, vec2(50, 50) /* initial x/y */, + vec2(25, -100) /* delta x/y */, + PointerChoreographerDisplayTopologyTestFixture::DISPLAY_TOP_ID, + vec2(50 + 25 - 10, + 90) /* Bottom edge: (source + delta - offset, height) */), + std::make_tuple("TransitionToBottomDisplay", + AINPUT_SOURCE_MOUSE | AINPUT_SOURCE_TOUCHPAD, ControllerType::MOUSE, + ToolType::FINGER, vec2(50, 50) /* initial x/y */, + vec2(25, 100) /* delta x/y */, + PointerChoreographerDisplayTopologyTestFixture::DISPLAY_BOTTOM_ID, + vec2(50 + 25 - 10, 0) /* Top edge: (source + delta - offset, 0) */), std::make_tuple("NoTransitionAtTopOffset", AINPUT_SOURCE_MOUSE, ControllerType::MOUSE, ToolType::MOUSE, - FloatPoint(5, 50) /* initial x/y */, - FloatPoint(0, -100) /* Move Up */, + vec2(5, 50) /* initial x/y */, vec2(0, -100) /* Move Up */, PointerChoreographerDisplayTopologyTestFixture::DISPLAY_CENTER_ID, - FloatPoint(5, 0) /* Top edge */), + vec2(5, 0) /* Top edge */), std::make_tuple("NoTransitionAtRightOffset", AINPUT_SOURCE_MOUSE, ControllerType::MOUSE, ToolType::MOUSE, - FloatPoint(95, 5) /* initial x/y */, - FloatPoint(100, 0) /* Move Right */, + vec2(95, 5) /* initial x/y */, vec2(100, 0) /* Move Right */, PointerChoreographerDisplayTopologyTestFixture::DISPLAY_CENTER_ID, - FloatPoint(99, 5) /* Top edge */), + vec2(99, 5) /* Top edge */), std::make_tuple("NoTransitionAtBottomOffset", AINPUT_SOURCE_MOUSE | AINPUT_SOURCE_TOUCHPAD, ControllerType::MOUSE, - ToolType::FINGER, FloatPoint(5, 95) /* initial x/y */, - FloatPoint(0, 100) /* Move Down */, + ToolType::FINGER, vec2(5, 95) /* initial x/y */, + vec2(0, 100) /* Move Down */, PointerChoreographerDisplayTopologyTestFixture::DISPLAY_CENTER_ID, - FloatPoint(5, 99) /* Bottom edge */), + vec2(5, 99) /* Bottom edge */), std::make_tuple("NoTransitionAtLeftOffset", AINPUT_SOURCE_MOUSE | AINPUT_SOURCE_TOUCHPAD, ControllerType::MOUSE, - ToolType::FINGER, FloatPoint(5, 5) /* initial x/y */, - FloatPoint(-100, 0) /* Move Left */, + ToolType::FINGER, vec2(5, 5) /* initial x/y */, + vec2(-100, 0) /* Move Left */, PointerChoreographerDisplayTopologyTestFixture::DISPLAY_CENTER_ID, - FloatPoint(0, 5) /* Left edge */), + vec2(0, 5) /* Left edge */), std::make_tuple( "TransitionAtTopRightCorner", AINPUT_SOURCE_MOUSE | AINPUT_SOURCE_TOUCHPAD, - ControllerType::MOUSE, ToolType::FINGER, - FloatPoint(95, 5) /* initial x/y */, - FloatPoint(10, -10) /* Move dignally to top right corner */, + ControllerType::MOUSE, ToolType::FINGER, vec2(95, 5) /* initial x/y */, + vec2(10, -10) /* Move dignally to top right corner */, PointerChoreographerDisplayTopologyTestFixture::DISPLAY_TOP_RIGHT_CORNER_ID, - FloatPoint(0, 90) /* bottom left corner */)), + vec2(0, 90) /* bottom left corner */)), [](const testing::TestParamInfo& p) { return std::string{std::get<0>(p.param)}; }); -- cgit v1.2.3-59-g8ed1b