From 15aafa78c9e87d3f8da5e3be817a0ead60b018f7 Mon Sep 17 00:00:00 2001 From: Aleksandr Litovchenko Date: Tue, 18 Jan 2022 20:51:50 +0000 Subject: [PATCH] Make LockPatternView dot hit area circular. Dot hit area is rectangular. Make it circular to be able to increase the area and preserve diagonal connections. Cover with tests to ensure that touches handled correctly. Bug: 204870856 Bug: 215183631 Tests: LockPatternViewTest and manualy on watch and phone Change-Id: I7d7b539afcdd4d86cb8f828030a2f4866963f8aa --- .../internal/widget/LockPatternView.java | 109 +++----- core/res/res/values/dimens.xml | 3 + core/res/res/values/symbols.xml | 1 + .../internal/widget/LockPatternViewTest.java | 247 ++++++++++++++++++ 4 files changed, 288 insertions(+), 72 deletions(-) create mode 100644 core/tests/coretests/src/com/android/internal/widget/LockPatternViewTest.java diff --git a/core/java/com/android/internal/widget/LockPatternView.java b/core/java/com/android/internal/widget/LockPatternView.java index 2b6b933c6886d..01cec7727c4ee 100644 --- a/core/java/com/android/internal/widget/LockPatternView.java +++ b/core/java/com/android/internal/widget/LockPatternView.java @@ -45,6 +45,7 @@ import android.util.AttributeSet; import android.util.IntArray; import android.util.Log; import android.util.SparseArray; +import android.util.TypedValue; import android.view.HapticFeedbackConstants; import android.view.MotionEvent; import android.view.RenderNodeAnimator; @@ -82,10 +83,12 @@ public class LockPatternView extends View { private static final int DOT_ACTIVATION_DURATION_MILLIS = 50; private static final int DOT_RADIUS_INCREASE_DURATION_MILLIS = 96; private static final int DOT_RADIUS_DECREASE_DURATION_MILLIS = 192; + private static final float MIN_DOT_HIT_FACTOR = 0.2f; private final CellState[][] mCellStates; private final int mDotSize; private final int mDotSizeActivated; + private final float mDotHitFactor; private final int mPathWidth; private boolean mDrawingProfilingStarted = false; @@ -143,12 +146,11 @@ public class LockPatternView extends View { private boolean mPatternInProgress = false; private boolean mFadePattern = true; - private float mHitFactor = 0.6f; - @UnsupportedAppUsage private float mSquareWidth; @UnsupportedAppUsage private float mSquareHeight; + private float mDotHitRadius; private final LinearGradient mFadeOutGradientShader; private final Path mCurrentPath = new Path(); @@ -164,8 +166,7 @@ public class LockPatternView extends View { private final Interpolator mFastOutSlowInInterpolator; private final Interpolator mLinearOutSlowInInterpolator; - private PatternExploreByTouchHelper mExploreByTouchHelper; - private AudioManager mAudioManager; + private final PatternExploreByTouchHelper mExploreByTouchHelper; private Drawable mSelectedDrawable; private Drawable mNotSelectedDrawable; @@ -349,6 +350,9 @@ public class LockPatternView extends View { mDotSize = getResources().getDimensionPixelSize(R.dimen.lock_pattern_dot_size); mDotSizeActivated = getResources().getDimensionPixelSize( R.dimen.lock_pattern_dot_size_activated); + TypedValue outValue = new TypedValue(); + getResources().getValue(R.dimen.lock_pattern_dot_hit_factor, outValue, true); + mDotHitFactor = Math.max(Math.min(outValue.getFloat(), 1f), MIN_DOT_HIT_FACTOR); mUseLockPatternDrawable = getResources().getBoolean(R.bool.use_lock_pattern_drawable); if (mUseLockPatternDrawable) { @@ -375,7 +379,6 @@ public class LockPatternView extends View { AnimationUtils.loadInterpolator(context, android.R.interpolator.linear_out_slow_in); mExploreByTouchHelper = new PatternExploreByTouchHelper(this); setAccessibilityDelegate(mExploreByTouchHelper); - mAudioManager = (AudioManager) mContext.getSystemService(Context.AUDIO_SERVICE); int fadeAwayGradientWidth = getResources().getDimensionPixelSize( R.dimen.lock_pattern_fade_away_gradient_width); @@ -679,6 +682,7 @@ public class LockPatternView extends View { final int height = h - mPaddingTop - mPaddingBottom; mSquareHeight = height / 3.0f; mExploreByTouchHelper.invalidateRoot(); + mDotHitRadius = Math.min(mSquareHeight / 2, mSquareWidth / 2) * mDotHitFactor; if (mUseLockPatternDrawable) { mNotSelectedDrawable.setBounds(mPaddingLeft, mPaddingTop, width, height); @@ -890,63 +894,30 @@ public class LockPatternView extends View { return set; } - // helper method to find which cell a point maps to + @Nullable private Cell checkForNewHit(float x, float y) { - - final int rowHit = getRowHit(y); - if (rowHit < 0) { - return null; + Cell cellHit = detectCellHit(x, y); + if (cellHit != null && !mPatternDrawLookup[cellHit.row][cellHit.column]) { + return cellHit; } - final int columnHit = getColumnHit(x); - if (columnHit < 0) { - return null; - } - - if (mPatternDrawLookup[rowHit][columnHit]) { - return null; - } - return Cell.of(rowHit, columnHit); + return null; } - /** - * Helper method to find the row that y falls into. - * @param y The y coordinate - * @return The row that y falls in, or -1 if it falls in no row. - */ - private int getRowHit(float y) { - - final float squareHeight = mSquareHeight; - float hitSize = squareHeight * mHitFactor; - - float offset = mPaddingTop + (squareHeight - hitSize) / 2f; - for (int i = 0; i < 3; i++) { - - final float hitTop = offset + squareHeight * i; - if (y >= hitTop && y <= hitTop + hitSize) { - return i; + /** Helper method to find which cell a point maps to. */ + @Nullable + private Cell detectCellHit(float x, float y) { + final float hitRadiusSquared = mDotHitRadius * mDotHitRadius; + for (int row = 0; row < 3; row++) { + for (int column = 0; column < 3; column++) { + float centerY = getCenterYForRow(row); + float centerX = getCenterXForColumn(column); + if ((x - centerX) * (x - centerX) + (y - centerY) * (y - centerY) + < hitRadiusSquared) { + return Cell.of(row, column); + } } } - return -1; - } - - /** - * Helper method to find the column x fallis into. - * @param x The x coordinate. - * @return The column that x falls in, or -1 if it falls in no column. - */ - private int getColumnHit(float x) { - final float squareWidth = mSquareWidth; - float hitSize = squareWidth * mHitFactor; - - float offset = mPaddingLeft + (squareWidth - hitSize) / 2f; - for (int i = 0; i < 3; i++) { - - final float hitLeft = offset + squareWidth * i; - if (x >= hitLeft && x <= hitLeft + hitSize) { - return i; - } - } - return -1; + return null; } @Override @@ -1553,8 +1524,7 @@ public class LockPatternView extends View { protected int getVirtualViewAt(float x, float y) { // This must use the same hit logic for the screen to ensure consistency whether // accessibility is on or off. - int id = getVirtualViewIdForHit(x, y); - return id; + return getVirtualViewIdForHit(x, y); } @Override @@ -1670,12 +1640,11 @@ public class LockPatternView extends View { final int col = ordinal % 3; float centerX = getCenterXForColumn(col); float centerY = getCenterYForRow(row); - float cellheight = mSquareHeight * mHitFactor * 0.5f; - float cellwidth = mSquareWidth * mHitFactor * 0.5f; - bounds.left = (int) (centerX - cellwidth); - bounds.right = (int) (centerX + cellwidth); - bounds.top = (int) (centerY - cellheight); - bounds.bottom = (int) (centerY + cellheight); + float cellHitRadius = mDotHitRadius; + bounds.left = (int) (centerX - cellHitRadius); + bounds.right = (int) (centerX + cellHitRadius); + bounds.top = (int) (centerY - cellHitRadius); + bounds.bottom = (int) (centerY + cellHitRadius); return bounds; } @@ -1694,16 +1663,12 @@ public class LockPatternView extends View { * @return VIRTUAL_BASE_VIEW_ID+id or 0 if no view was hit */ private int getVirtualViewIdForHit(float x, float y) { - final int rowHit = getRowHit(y); - if (rowHit < 0) { + Cell cellHit = detectCellHit(x, y); + if (cellHit == null) { return ExploreByTouchHelper.INVALID_ID; } - final int columnHit = getColumnHit(x); - if (columnHit < 0) { - return ExploreByTouchHelper.INVALID_ID; - } - boolean dotAvailable = mPatternDrawLookup[rowHit][columnHit]; - int dotId = (rowHit * 3 + columnHit) + VIRTUAL_BASE_VIEW_ID; + boolean dotAvailable = mPatternDrawLookup[cellHit.row][cellHit.column]; + int dotId = (cellHit.row * 3 + cellHit.column) + VIRTUAL_BASE_VIEW_ID; int view = dotAvailable ? dotId : ExploreByTouchHelper.INVALID_ID; if (DEBUG_A11Y) Log.v(TAG, "getVirtualViewIdForHit(" + x + "," + y + ") => " + view + "avail =" + dotAvailable); diff --git a/core/res/res/values/dimens.xml b/core/res/res/values/dimens.xml index 1b9f7feec0ef0..44c5512593750 100644 --- a/core/res/res/values/dimens.xml +++ b/core/res/res/values/dimens.xml @@ -668,6 +668,9 @@ 22dp 14dp 30dp + + 0.6 8dp diff --git a/core/res/res/values/symbols.xml b/core/res/res/values/symbols.xml index 8f3abd60dd3ca..6f34b3f899f12 100644 --- a/core/res/res/values/symbols.xml +++ b/core/res/res/values/symbols.xml @@ -1325,6 +1325,7 @@ + diff --git a/core/tests/coretests/src/com/android/internal/widget/LockPatternViewTest.java b/core/tests/coretests/src/com/android/internal/widget/LockPatternViewTest.java new file mode 100644 index 0000000000000..8ba49663ad099 --- /dev/null +++ b/core/tests/coretests/src/com/android/internal/widget/LockPatternViewTest.java @@ -0,0 +1,247 @@ +/* + * Copyright (C) 2022 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 com.android.internal.widget; + +import static android.view.ViewGroup.LayoutParams.MATCH_PARENT; + +import android.content.Context; + +import androidx.test.annotation.UiThreadTest; + +import android.util.AttributeSet; +import android.view.MotionEvent; +import android.view.View; +import android.view.ViewGroup; +import android.widget.Toolbar; + + +import static org.hamcrest.Matchers.contains; +import static org.hamcrest.Matchers.hasSize; +import static org.hamcrest.Matchers.is; +import static org.junit.Assert.assertThat; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.ArgumentMatchers.eq; +import static org.mockito.Mockito.atLeastOnce; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.never; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.verifyNoMoreInteractions; + +import androidx.test.InstrumentationRegistry; +import androidx.test.filters.SmallTest; +import androidx.test.rule.UiThreadTestRule; + +import org.junit.Before; +import org.junit.Rule; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.junit.runners.Parameterized; +import org.mockito.ArgumentCaptor; +import org.mockito.ArgumentMatchers; +import org.mockito.Captor; +import org.mockito.Mock; +import org.mockito.MockitoAnnotations; + +import com.android.internal.R; + +import java.util.Arrays; +import java.util.Collection; +import java.util.List; + +@RunWith(Parameterized.class) +@SmallTest +public class LockPatternViewTest { + + @Rule + public UiThreadTestRule uiThreadTestRule = new UiThreadTestRule(); + + private final int mViewSize; + private final float mDefaultError; + private final float mDot1x; + private final float mDot1y; + private final float mDot2x; + private final float mDot2y; + private final float mDot3x; + private final float mDot3y; + private final float mDot5x; + private final float mDot5y; + private final float mDot7x; + private final float mDot7y; + private final float mDot9x; + private final float mDot9y; + + private Context mContext; + private LockPatternView mLockPatternView; + @Mock + private LockPatternView.OnPatternListener mPatternListener; + @Captor + private ArgumentCaptor> mCellsArgumentCaptor; + + public LockPatternViewTest(int viewSize) { + mViewSize = viewSize; + float cellSize = viewSize / 3f; + mDefaultError = cellSize * 0.2f; + mDot1x = cellSize / 2f; + mDot1y = cellSize / 2f; + mDot2x = cellSize + mDot1x; + mDot2y = mDot1y; + mDot3x = cellSize + mDot2x; + mDot3y = mDot1y; + // dot4 is skipped as redundant + mDot5x = cellSize + mDot1x; + mDot5y = cellSize + mDot1y; + // dot6 is skipped as redundant + mDot7x = mDot1x; + mDot7y = cellSize * 2 + mDot1y; + // dot8 is skipped as redundant + mDot9x = cellSize * 2 + mDot7x; + mDot9y = mDot7y; + } + + @Parameterized.Parameters + public static Collection primeNumbers() { + return Arrays.asList(192, 512, 768, 1024); + } + + @Before + public void setUp() throws Exception { + MockitoAnnotations.initMocks(this); + mContext = InstrumentationRegistry.getContext(); + mLockPatternView = new LockPatternView(mContext, null); + int heightMeasureSpec = View.MeasureSpec.makeMeasureSpec(mViewSize, + View.MeasureSpec.EXACTLY); + int widthMeasureSpec = View.MeasureSpec.makeMeasureSpec(mViewSize, + View.MeasureSpec.EXACTLY); + mLockPatternView.measure(widthMeasureSpec, heightMeasureSpec); + mLockPatternView.layout(0, 0, mLockPatternView.getMeasuredWidth(), + mLockPatternView.getMeasuredHeight()); + } + + @UiThreadTest + @Test + public void downStartsPattern() { + mLockPatternView.setOnPatternListener(mPatternListener); + mLockPatternView.onTouchEvent( + MotionEvent.obtain(0, 0, MotionEvent.ACTION_DOWN, mDot1x, mDot1y, 1)); + verify(mPatternListener).onPatternStart(); + } + + @UiThreadTest + @Test + public void up_completesPattern() { + mLockPatternView.setOnPatternListener(mPatternListener); + mLockPatternView.onTouchEvent( + MotionEvent.obtain(0, 0, MotionEvent.ACTION_DOWN, mDot1x, mDot1y, 1)); + mLockPatternView.onTouchEvent( + MotionEvent.obtain(0, 0, MotionEvent.ACTION_UP, mDot1x, mDot1y, 1)); + verify(mPatternListener).onPatternDetected(any()); + } + + @UiThreadTest + @Test + public void moveToDot_hitsDot() { + mLockPatternView.setOnPatternListener(mPatternListener); + mLockPatternView.onTouchEvent( + MotionEvent.obtain(0, 0, MotionEvent.ACTION_DOWN, 1f, 1f, 1)); + mLockPatternView.onTouchEvent( + MotionEvent.obtain(0, 0, MotionEvent.ACTION_MOVE, mDot1x, mDot1y, 1)); + verify(mPatternListener).onPatternStart(); + } + + @UiThreadTest + @Test + public void moveOutside_doesNotHitsDot() { + mLockPatternView.setOnPatternListener(mPatternListener); + mLockPatternView.onTouchEvent( + MotionEvent.obtain(0, 0, MotionEvent.ACTION_DOWN, 1f, 1f, 1)); + mLockPatternView.onTouchEvent( + MotionEvent.obtain(0, 0, MotionEvent.ACTION_MOVE, 2f, 2f, 1)); + verify(mPatternListener, never()).onPatternStart(); + } + + @UiThreadTest + @Test + public void moveAlongTwoDots_hitsTwo() { + mLockPatternView.setOnPatternListener(mPatternListener); + mLockPatternView.onTouchEvent( + MotionEvent.obtain(0, 0, MotionEvent.ACTION_DOWN, 1f, 1f, 1)); + makeMove(mDot1x, mDot1y, mDot2x, mDot2y, 6); + mLockPatternView.onTouchEvent( + MotionEvent.obtain(0, 3, MotionEvent.ACTION_UP, mDot2x, mDot2y, 1)); + + verify(mPatternListener).onPatternDetected(mCellsArgumentCaptor.capture()); + List patternCells = mCellsArgumentCaptor.getValue(); + assertThat(patternCells, hasSize(2)); + assertThat(patternCells, + contains(LockPatternView.Cell.of(0, 0), LockPatternView.Cell.of(0, 1))); + } + + @UiThreadTest + @Test + public void moveAlongTwoDotsDiagonally_hitsTwo() { + mLockPatternView.setOnPatternListener(mPatternListener); + mLockPatternView.onTouchEvent( + MotionEvent.obtain(0, 0, MotionEvent.ACTION_DOWN, 1f, 1f, 1)); + makeMove(mDot1x, mDot1y, mDot5x, mDot5y, 6); + mLockPatternView.onTouchEvent( + MotionEvent.obtain(0, 3, MotionEvent.ACTION_UP, mDot5x, mDot5y, 1)); + + verify(mPatternListener).onPatternDetected(mCellsArgumentCaptor.capture()); + List patternCells = mCellsArgumentCaptor.getValue(); + assertThat(patternCells, hasSize(2)); + assertThat(patternCells, + contains(LockPatternView.Cell.of(0, 0), LockPatternView.Cell.of(1, 1))); + } + + @UiThreadTest + @Test + public void moveAlongZPattern_hitsDots() { + mLockPatternView.setOnPatternListener(mPatternListener); + mLockPatternView.onTouchEvent( + MotionEvent.obtain(0, 0, MotionEvent.ACTION_DOWN, 1f, 1f, 1)); + makeMove(mDot1x, mDot1y, mDot3x + mDefaultError, mDot3y, 10); + makeMove(mDot3x - mDefaultError, mDot3y, mDot7x, mDot7y, 10); + makeMove(mDot7x, mDot7y - mDefaultError, mDot9x, mDot9y - mDefaultError, 10); + mLockPatternView.onTouchEvent( + MotionEvent.obtain(0, 0, MotionEvent.ACTION_UP, mViewSize - mDefaultError, + mViewSize - mDefaultError, 1)); + + verify(mPatternListener).onPatternDetected(mCellsArgumentCaptor.capture()); + List patternCells = mCellsArgumentCaptor.getValue(); + assertThat(patternCells, hasSize(7)); + assertThat(patternCells, + contains(LockPatternView.Cell.of(0, 0), + LockPatternView.Cell.of(0, 1), + LockPatternView.Cell.of(0, 2), + LockPatternView.Cell.of(1, 1), + LockPatternView.Cell.of(2, 0), + LockPatternView.Cell.of(2, 1), + LockPatternView.Cell.of(2, 2))); + } + + private void makeMove(float xFrom, float yFrom, float xTo, float yTo, int numberOfSteps) { + for (int i = 0; i < numberOfSteps; i++) { + float progress = i / (numberOfSteps - 1f); + float rest = 1f - progress; + mLockPatternView.onTouchEvent( + MotionEvent.obtain(0, 0, MotionEvent.ACTION_MOVE, + /* x= */ xFrom * rest + xTo * progress, + /* y= */ yFrom * rest + yTo * progress, + 1)); + } + } +}