From 2bd253a08341bc5793e3a255888dededcafbd661 Mon Sep 17 00:00:00 2001 From: Grace Cheng Date: Thu, 7 Apr 2022 21:30:03 +0000 Subject: [PATCH] Fix location of side-fps indicator 1. Corrects the calculation for side-fps indicator coordinates and cleans up rotation logic 2. Removes premature & duplicate call to updateOverlayParams 3. Fixes updateOverlayParams call to avoid race condition, and prevent updates on stale view reference 4. Updates AuthController to fetch display width correctly 5. Remove faulty tests and add additional tests to verify indicator placement Fixes: 218701671 Test: Observe location of indicator when rotating through all 4 orientations / folded & unfolded configurations on all side-fps devices Test: atest SidefpsControllerTest Change-Id: Ieacc8ba1f60335c632e6b53d8b3c9d1131c71bc4 --- .../systemui/biometrics/AuthController.java | 9 +- .../systemui/biometrics/SidefpsController.kt | 88 ++++++------- .../biometrics/SidefpsControllerTest.kt | 124 +++++++++++++----- 3 files changed, 138 insertions(+), 83 deletions(-) diff --git a/packages/SystemUI/src/com/android/systemui/biometrics/AuthController.java b/packages/SystemUI/src/com/android/systemui/biometrics/AuthController.java index d4dad73a49f2e..002b30d26654c 100644 --- a/packages/SystemUI/src/com/android/systemui/biometrics/AuthController.java +++ b/packages/SystemUI/src/com/android/systemui/biometrics/AuthController.java @@ -65,7 +65,6 @@ import com.android.internal.annotations.VisibleForTesting; import com.android.internal.os.SomeArgs; import com.android.internal.widget.LockPatternUtils; import com.android.systemui.CoreStartable; -import com.android.systemui.assist.ui.DisplayUtils; import com.android.systemui.dagger.SysUISingleton; import com.android.systemui.dagger.qualifiers.Background; import com.android.systemui.dagger.qualifiers.Main; @@ -579,8 +578,14 @@ public class AuthController extends CoreStartable implements CommandQueue.Callba mSensorPrivacyManager = context.getSystemService(SensorPrivacyManager.class); } + private int getDisplayWidth() { + DisplayInfo displayInfo = new DisplayInfo(); + mContext.getDisplay().getDisplayInfo(displayInfo); + return displayInfo.getNaturalWidth(); + } + private void updateFingerprintLocation() { - int xLocation = DisplayUtils.getWidth(mContext) / 2; + int xLocation = getDisplayWidth() / 2; try { xLocation = mContext.getResources().getDimensionPixelSize( com.android.systemui.R.dimen diff --git a/packages/SystemUI/src/com/android/systemui/biometrics/SidefpsController.kt b/packages/SystemUI/src/com/android/systemui/biometrics/SidefpsController.kt index 085bcfafdf784..04e2dccda5284 100644 --- a/packages/SystemUI/src/com/android/systemui/biometrics/SidefpsController.kt +++ b/packages/SystemUI/src/com/android/systemui/biometrics/SidefpsController.kt @@ -33,6 +33,7 @@ import android.hardware.fingerprint.FingerprintSensorPropertiesInternal import android.hardware.fingerprint.ISidefpsController import android.os.Handler import android.util.Log +import android.util.RotationUtils import android.view.View.AccessibilityDelegate import android.view.accessibility.AccessibilityEvent import android.view.Display @@ -116,7 +117,8 @@ class SidefpsController @Inject constructor( orientationListener.enable() } } - private var overlayOffsets: SensorLocationInternal = SensorLocationInternal.DEFAULT + @VisibleForTesting + internal var overlayOffsets: SensorLocationInternal = SensorLocationInternal.DEFAULT private val overlayViewParams = WindowManager.LayoutParams( WindowManager.LayoutParams.WRAP_CONTENT, @@ -141,7 +143,7 @@ class SidefpsController @Inject constructor( private fun doShow() = mainExecutor.execute { if (overlayView == null) { - overlayView = createOverlayForDisplay() + createOverlayForDisplay() } else { Log.v(TAG, "overlay already shown") } @@ -154,14 +156,14 @@ class SidefpsController @Inject constructor( private fun onOrientationChanged() { if (overlayView != null) { - overlayView = createOverlayForDisplay() + createOverlayForDisplay() } } - private fun createOverlayForDisplay(): View { + private fun createOverlayForDisplay() { val view = layoutInflater.inflate(R.layout.sidefps_view, null, false) + overlayView = view val display = context.display!! - val offsets = sensorProps.getLocation(display.uniqueId).let { location -> if (location == null) { Log.w(TAG, "No location specified for display: ${display.uniqueId}") @@ -172,13 +174,11 @@ class SidefpsController @Inject constructor( val lottie = view.findViewById(R.id.sidefps_animation) as LottieAnimationView view.rotation = display.asSideFpsAnimationRotation(offsets.isYAligned()) - - updateOverlayParams(display, lottie.composition?.bounds ?: Rect()) lottie.setAnimation(display.asSideFpsAnimation(offsets.isYAligned())) lottie.addLottieOnCompositionLoadedListener { - if (overlayView == view) { + // Check that view is not stale, and that overlayView has not been hidden/removed + if (overlayView != null && overlayView == view) { updateOverlayParams(display, it.bounds) - windowManager.updateViewLayout(overlayView, overlayViewParams) } } lottie.addOverlayDynamicColor(context) @@ -200,55 +200,47 @@ class SidefpsController @Inject constructor( } } }) - return view } - private fun updateOverlayParams(display: Display, bounds: Rect) { - val isPortrait = display.isPortrait() + @VisibleForTesting + internal fun updateOverlayParams(display: Display, bounds: Rect) { + val isNaturalOrientation = display.isNaturalOrientation() val size = windowManager.maximumWindowMetrics.bounds - val displayWidth = if (isPortrait) size.width() else size.height() - val displayHeight = if (isPortrait) size.height() else size.width() - - // ignore sensorRadius since it's assumed that the sensor is on the side and centered at - // either sensorLocationX or sensorLocationY (both should not be set) - val (x, y) = if (overlayOffsets.isYAligned()) { - when (display.rotation) { - Surface.ROTATION_90 -> - Pair(overlayOffsets.sensorLocationY, 0) - Surface.ROTATION_270 -> - Pair( - displayHeight - overlayOffsets.sensorLocationY - bounds.width(), - displayWidth + bounds.height() - ) - Surface.ROTATION_180 -> - Pair(0, displayHeight - overlayOffsets.sensorLocationY - bounds.height()) - else -> - Pair(displayWidth, overlayOffsets.sensorLocationY) - } + val displayWidth = if (isNaturalOrientation) size.width() else size.height() + val displayHeight = if (isNaturalOrientation) size.height() else size.width() + val boundsWidth = if (isNaturalOrientation) bounds.width() else bounds.height() + val boundsHeight = if (isNaturalOrientation) bounds.height() else bounds.width() + val sensorBounds = if (overlayOffsets.isYAligned()) { + Rect( + displayWidth - boundsWidth, + overlayOffsets.sensorLocationY, + displayWidth, + overlayOffsets.sensorLocationY + boundsHeight + ) } else { - when (display.rotation) { - Surface.ROTATION_90 -> - Pair(0, displayWidth - overlayOffsets.sensorLocationX - bounds.height()) - Surface.ROTATION_270 -> - Pair(displayWidth, overlayOffsets.sensorLocationX - bounds.height()) - Surface.ROTATION_180 -> - Pair( - displayWidth - overlayOffsets.sensorLocationX - bounds.width(), - displayHeight - ) - else -> - Pair(overlayOffsets.sensorLocationX, 0) - } + Rect( + overlayOffsets.sensorLocationX, + 0, + overlayOffsets.sensorLocationX + boundsWidth, + boundsHeight + ) } - overlayViewParams.x = x - overlayViewParams.y = y + + RotationUtils.rotateBounds( + sensorBounds, + Rect(0, 0, displayWidth, displayHeight), + display.rotation + ) + + overlayViewParams.x = sensorBounds.left + overlayViewParams.y = sensorBounds.top + windowManager.updateViewLayout(overlayView, overlayViewParams) } private fun updateOverlayVisibility(view: View) { if (view != overlayView) { return } - // hide after a few seconds if the sensor is oriented down and there are // large overlapping system bars val rotation = context.display?.rotation @@ -304,7 +296,7 @@ private fun Display.asSideFpsAnimationRotation(yAligned: Boolean): Float = when private fun SensorLocationInternal.isYAligned(): Boolean = sensorLocationY != 0 -private fun Display.isPortrait(): Boolean = +private fun Display.isNaturalOrientation(): Boolean = rotation == Surface.ROTATION_0 || rotation == Surface.ROTATION_180 private fun WindowInsets.hasBigNavigationBar(): Boolean = diff --git a/packages/SystemUI/tests/src/com/android/systemui/biometrics/SidefpsControllerTest.kt b/packages/SystemUI/tests/src/com/android/systemui/biometrics/SidefpsControllerTest.kt index e1a348ead9b77..102f37c4a0376 100644 --- a/packages/SystemUI/tests/src/com/android/systemui/biometrics/SidefpsControllerTest.kt +++ b/packages/SystemUI/tests/src/com/android/systemui/biometrics/SidefpsControllerTest.kt @@ -77,12 +77,6 @@ import org.mockito.junit.MockitoJUnit private const val DISPLAY_ID = 2 private const val SENSOR_ID = 1 -private const val DISPLAY_SIZE_X = 800 -private const val DISPLAY_SIZE_Y = 900 - -private val X_LOCATION = SensorLocationInternal("", 540, 0, 20) -private val Y_LOCATION = SensorLocationInternal("", 0, 1500, 22) - @SmallTest @RunWith(AndroidTestingRunner::class) @TestableLooper.RunWithLooper @@ -116,6 +110,17 @@ class SidefpsControllerTest : SysuiTestCase() { private lateinit var overlayController: ISidefpsController private lateinit var sideFpsController: SidefpsController + enum class DeviceConfig { X_ALIGNED, Y_ALIGNED_UNFOLDED, Y_ALIGNED_FOLDED } + + private lateinit var deviceConfig: DeviceConfig + private lateinit var indicatorBounds: Rect + private lateinit var displayBounds: Rect + private lateinit var sensorLocation: SensorLocationInternal + private var displayWidth: Int = 0 + private var displayHeight: Int = 0 + private var boundsWidth: Int = 0 + private var boundsHeight: Int = 0 + @Before fun setup() { context.addMockSystemService(DisplayManager::class.java, displayManager) @@ -135,17 +140,43 @@ class SidefpsControllerTest : SysuiTestCase() { this } } - `when`(windowManager.maximumWindowMetrics).thenReturn( - WindowMetrics(Rect(0, 0, DISPLAY_SIZE_X, DISPLAY_SIZE_Y), WindowInsets.CONSUMED) - ) } private fun testWithDisplay( + deviceConfig: DeviceConfig = DeviceConfig.X_ALIGNED, initInfo: DisplayInfo.() -> Unit = {}, - locations: List = listOf(X_LOCATION), windowInsets: WindowInsets = insetsForSmallNavbar(), block: () -> Unit ) { + this.deviceConfig = deviceConfig + + when (deviceConfig) { + DeviceConfig.X_ALIGNED -> { + displayWidth = 2560 + displayHeight = 1600 + sensorLocation = SensorLocationInternal("", 2325, 0, 0) + boundsWidth = 160 + boundsHeight = 84 + } + DeviceConfig.Y_ALIGNED_UNFOLDED -> { + displayWidth = 2208 + displayHeight = 1840 + sensorLocation = SensorLocationInternal("", 0, 510, 0) + boundsWidth = 110 + boundsHeight = 210 + } + DeviceConfig.Y_ALIGNED_FOLDED -> { + displayWidth = 1080 + displayHeight = 2100 + sensorLocation = SensorLocationInternal("", 0, 590, 0) + boundsWidth = 110 + boundsHeight = 210 + } + } + indicatorBounds = Rect(0, 0, boundsWidth, boundsHeight) + displayBounds = Rect(0, 0, displayWidth, displayHeight) + var locations = listOf(sensorLocation) + `when`(fingerprintManager.sensorPropertiesInternal).thenReturn( listOf( FingerprintSensorPropertiesInternal( @@ -166,8 +197,11 @@ class SidefpsControllerTest : SysuiTestCase() { val display = Display(dmGlobal, DISPLAY_ID, displayInfo, DEFAULT_DISPLAY_ADJUSTMENTS) `when`(dmGlobal.getDisplayInfo(eq(DISPLAY_ID))).thenReturn(displayInfo) `when`(windowManager.defaultDisplay).thenReturn(display) + `when`(windowManager.maximumWindowMetrics).thenReturn( + WindowMetrics(displayBounds, WindowInsets.CONSUMED) + ) `when`(windowManager.currentWindowMetrics).thenReturn( - WindowMetrics(Rect(0, 0, DISPLAY_SIZE_X, DISPLAY_SIZE_Y), windowInsets) + WindowMetrics(displayBounds, windowInsets) ) sideFpsController = SidefpsController( @@ -260,46 +294,56 @@ class SidefpsControllerTest : SysuiTestCase() { } @Test - fun showsWithTaskbar() = testWithDisplay({ rotation = Surface.ROTATION_0 }) { + fun showsWithTaskbar() = testWithDisplay( + deviceConfig = DeviceConfig.X_ALIGNED, + { rotation = Surface.ROTATION_0 } + ) { hidesWithTaskbar(visible = true) } @Test fun showsWithTaskbarOnY() = testWithDisplay( - { rotation = Surface.ROTATION_0 }, - locations = listOf(Y_LOCATION) + deviceConfig = DeviceConfig.Y_ALIGNED_UNFOLDED, + { rotation = Surface.ROTATION_0 } ) { hidesWithTaskbar(visible = true) } @Test - fun showsWithTaskbar90() = testWithDisplay({ rotation = Surface.ROTATION_90 }) { + fun showsWithTaskbar90() = testWithDisplay( + deviceConfig = DeviceConfig.X_ALIGNED, + { rotation = Surface.ROTATION_90 } + ) { hidesWithTaskbar(visible = true) } @Test fun showsWithTaskbar90OnY() = testWithDisplay( - { rotation = Surface.ROTATION_90 }, - locations = listOf(Y_LOCATION) + deviceConfig = DeviceConfig.Y_ALIGNED_UNFOLDED, + { rotation = Surface.ROTATION_90 } ) { hidesWithTaskbar(visible = true) } @Test - fun showsWithTaskbar180() = testWithDisplay({ rotation = Surface.ROTATION_180 }) { + fun showsWithTaskbar180() = testWithDisplay( + deviceConfig = DeviceConfig.X_ALIGNED, + { rotation = Surface.ROTATION_180 } + ) { hidesWithTaskbar(visible = true) } @Test fun showsWithTaskbar270OnY() = testWithDisplay( - { rotation = Surface.ROTATION_270 }, - locations = listOf(Y_LOCATION) + deviceConfig = DeviceConfig.Y_ALIGNED_UNFOLDED, + { rotation = Surface.ROTATION_270 } ) { hidesWithTaskbar(visible = true) } @Test fun showsWithTaskbarCollapsedDown() = testWithDisplay( + deviceConfig = DeviceConfig.X_ALIGNED, { rotation = Surface.ROTATION_270 }, windowInsets = insetsForSmallNavbar() ) { @@ -308,8 +352,8 @@ class SidefpsControllerTest : SysuiTestCase() { @Test fun showsWithTaskbarCollapsedDownOnY() = testWithDisplay( + deviceConfig = DeviceConfig.Y_ALIGNED_UNFOLDED, { rotation = Surface.ROTATION_180 }, - locations = listOf(Y_LOCATION), windowInsets = insetsForSmallNavbar() ) { hidesWithTaskbar(visible = true) @@ -317,8 +361,8 @@ class SidefpsControllerTest : SysuiTestCase() { @Test fun hidesWithTaskbarDown() = testWithDisplay( + deviceConfig = DeviceConfig.X_ALIGNED, { rotation = Surface.ROTATION_180 }, - locations = listOf(X_LOCATION), windowInsets = insetsForLargeNavbar() ) { hidesWithTaskbar(visible = false) @@ -326,18 +370,18 @@ class SidefpsControllerTest : SysuiTestCase() { @Test fun hidesWithTaskbarDownOnY() = testWithDisplay( + deviceConfig = DeviceConfig.Y_ALIGNED_UNFOLDED, { rotation = Surface.ROTATION_270 }, - locations = listOf(Y_LOCATION), windowInsets = insetsForLargeNavbar() ) { - hidesWithTaskbar(visible = false) + hidesWithTaskbar(visible = true) } private fun hidesWithTaskbar(visible: Boolean) { overlayController.show(SENSOR_ID, REASON_UNKNOWN) executor.runAllReady() - sideFpsController.overviewProxyListener.onTaskbarStatusUpdated(true, false) + sideFpsController.overviewProxyListener.onTaskbarStatusUpdated(visible, false) executor.runAllReady() verify(windowManager).addView(any(), any()) @@ -346,25 +390,38 @@ class SidefpsControllerTest : SysuiTestCase() { } @Test - fun setsXAlign() = testWithDisplay { + fun testIndicatorPlacementForXAlignedSensor() = testWithDisplay( + deviceConfig = DeviceConfig.X_ALIGNED + ) { overlayController.show(SENSOR_ID, REASON_UNKNOWN) + sideFpsController.overlayOffsets = sensorLocation + sideFpsController.updateOverlayParams( + windowManager.defaultDisplay, + indicatorBounds + ) executor.runAllReady() - verify(windowManager).addView(any(), overlayViewParamsCaptor.capture()) + verify(windowManager).updateViewLayout(any(), overlayViewParamsCaptor.capture()) - assertThat(overlayViewParamsCaptor.value.x).isEqualTo(X_LOCATION.sensorLocationX) + assertThat(overlayViewParamsCaptor.value.x).isEqualTo(sensorLocation.sensorLocationX) assertThat(overlayViewParamsCaptor.value.y).isEqualTo(0) } @Test - fun setYAlign() = testWithDisplay(locations = listOf(Y_LOCATION)) { + fun testIndicatorPlacementForYAlignedSensor() = testWithDisplay( + deviceConfig = DeviceConfig.Y_ALIGNED_UNFOLDED + ) { + sideFpsController.overlayOffsets = sensorLocation + sideFpsController.updateOverlayParams( + windowManager.defaultDisplay, + indicatorBounds + ) overlayController.show(SENSOR_ID, REASON_UNKNOWN) executor.runAllReady() - verify(windowManager).addView(any(), overlayViewParamsCaptor.capture()) - - assertThat(overlayViewParamsCaptor.value.x).isEqualTo(DISPLAY_SIZE_X) - assertThat(overlayViewParamsCaptor.value.y).isEqualTo(Y_LOCATION.sensorLocationY) + verify(windowManager).updateViewLayout(any(), overlayViewParamsCaptor.capture()) + assertThat(overlayViewParamsCaptor.value.x).isEqualTo(displayWidth - boundsWidth) + assertThat(overlayViewParamsCaptor.value.y).isEqualTo(sensorLocation.sensorLocationY) } } @@ -373,6 +430,7 @@ private fun insetsForLargeNavbar() = insetsWithBottom(100) private fun insetsWithBottom(bottom: Int) = WindowInsets.Builder() .setInsets(WindowInsets.Type.navigationBars(), Insets.of(0, 0, 0, bottom)) .build() + private fun fpEnrollTask() = settingsTask(".biometrics.fingerprint.FingerprintEnrollEnrolling") private fun fpSettingsTask() = settingsTask(".biometrics.fingerprint.FingerprintSettings") private fun settingsTask(cls: String) = ActivityManager.RunningTaskInfo().apply {