From 8ccd7eb5388272d817f65c248619ea94413948dc Mon Sep 17 00:00:00 2001 From: Matt Casey Date: Wed, 7 Apr 2021 12:21:21 -0400 Subject: [PATCH 1/2] TalkBack and switch access support for CropView - Switch to androidx ExploreByTouchHelper, add View pass-through calls. - Add virtual view IDs for left/right bounds as well. - Make content description describe the position (as a %) - Allow volume buttons to move the current boundary along its axis (matching pattern used in Photos) - Remove unused click action for boundaries (was breaking switch access) Bug: 184018281 Bug: 184012364 Bug: 180062481 Bug: 184105678 Test: Enable talkback, move crop bounds with volume keys, same with switch access Change-Id: I06bab02a37620f19e15cddcfc79ae858a1614e92 --- packages/SystemUI/res/values/strings.xml | 12 +- .../android/systemui/screenshot/CropView.java | 213 +++++++++++++----- 2 files changed, 161 insertions(+), 64 deletions(-) diff --git a/packages/SystemUI/res/values/strings.xml b/packages/SystemUI/res/values/strings.xml index fba62691c951b..bed7442d576e5 100644 --- a/packages/SystemUI/res/values/strings.xml +++ b/packages/SystemUI/res/values/strings.xml @@ -245,10 +245,14 @@ Dismiss screenshot Screenshot preview - - Top boundary - - Bottom boundary + + Top boundary %1$d percent + + Bottom boundary %1$d percent + + Left boundary %1$d percent + + Right boundary %1$d percent Screen Recorder diff --git a/packages/SystemUI/src/com/android/systemui/screenshot/CropView.java b/packages/SystemUI/src/com/android/systemui/screenshot/CropView.java index 78ee896075703..bbce7b09e9b67 100644 --- a/packages/SystemUI/src/com/android/systemui/screenshot/CropView.java +++ b/packages/SystemUI/src/com/android/systemui/screenshot/CropView.java @@ -28,21 +28,26 @@ import android.os.Bundle; import android.os.Parcel; import android.os.Parcelable; import android.util.AttributeSet; -import android.util.IntArray; import android.util.Log; import android.util.MathUtils; import android.util.Range; +import android.view.KeyEvent; import android.view.MotionEvent; import android.view.View; import android.view.accessibility.AccessibilityEvent; import android.view.accessibility.AccessibilityNodeInfo; +import android.widget.SeekBar; import androidx.annotation.Nullable; +import androidx.core.view.ViewCompat; +import androidx.core.view.accessibility.AccessibilityNodeInfoCompat; +import androidx.customview.widget.ExploreByTouchHelper; import androidx.interpolator.view.animation.FastOutSlowInInterpolator; -import com.android.internal.widget.ExploreByTouchHelper; import com.android.systemui.R; +import java.util.List; + /** * CropView has top and bottom draggable crop handles, with a scrim to darken the areas being * cropped out. @@ -74,6 +79,7 @@ public class CropView extends View { private Range mMotionRange; private CropInteractionListener mCropInteractionListener; + private final ExploreByTouchHelper mExploreByTouchHelper; public CropView(Context context, @Nullable AttributeSet attrs) { this(context, attrs, 0); @@ -94,7 +100,8 @@ public class CropView extends View { // 48 dp touchable region around each handle. mCropTouchMargin = 24 * getResources().getDisplayMetrics().density; - setAccessibilityDelegate(new AccessibilityHelper()); + mExploreByTouchHelper = new AccessibilityHelper(); + ViewCompat.setAccessibilityDelegate(this, mExploreByTouchHelper); } @Override @@ -141,28 +148,7 @@ public class CropView extends View { mStartingX = event.getX(); mMovementStartValue = getBoundaryPosition(mCurrentDraggingBoundary); updateListener(event); - switch (mCurrentDraggingBoundary) { - case TOP: - mMotionRange = new Range<>(0f, - mCrop.bottom - pixelDistanceToFraction(mCropTouchMargin, - CropBoundary.BOTTOM)); - break; - case BOTTOM: - mMotionRange = new Range<>( - mCrop.top + pixelDistanceToFraction(mCropTouchMargin, - CropBoundary.TOP), 1f); - break; - case LEFT: - mMotionRange = new Range<>(0f, - mCrop.right - pixelDistanceToFraction(mCropTouchMargin, - CropBoundary.RIGHT)); - break; - case RIGHT: - mMotionRange = new Range<>( - mCrop.left + pixelDistanceToFraction(mCropTouchMargin, - CropBoundary.LEFT), 1f); - break; - } + mMotionRange = getAllowedValues(mCurrentDraggingBoundary); } return true; case MotionEvent.ACTION_MOVE: @@ -185,10 +171,30 @@ public class CropView extends View { return super.onTouchEvent(event); } + @Override + public boolean dispatchHoverEvent(MotionEvent event) { + return mExploreByTouchHelper.dispatchHoverEvent(event) + || super.dispatchHoverEvent(event); + } + + @Override + public boolean dispatchKeyEvent(KeyEvent event) { + return mExploreByTouchHelper.dispatchKeyEvent(event) + || super.dispatchKeyEvent(event); + } + + @Override + public void onFocusChanged(boolean gainFocus, int direction, + Rect previouslyFocusedRect) { + super.onFocusChanged(gainFocus, direction, previouslyFocusedRect); + mExploreByTouchHelper.onFocusChanged(gainFocus, direction, previouslyFocusedRect); + } + /** * Set the given boundary to the given value without animation. */ public void setBoundaryPosition(CropBoundary boundary, float position) { + position = (float) getAllowedValues(boundary).clamp(position); switch (boundary) { case TOP: mCrop.top = position; @@ -280,6 +286,28 @@ public class CropView extends View { mCropInteractionListener = listener; } + private Range getAllowedValues(CropBoundary boundary) { + switch (boundary) { + case TOP: + return new Range<>(0f, + mCrop.bottom - pixelDistanceToFraction(mCropTouchMargin, + CropBoundary.BOTTOM)); + case BOTTOM: + return new Range<>( + mCrop.top + pixelDistanceToFraction(mCropTouchMargin, + CropBoundary.TOP), 1f); + case LEFT: + return new Range<>(0f, + mCrop.right - pixelDistanceToFraction(mCropTouchMargin, + CropBoundary.RIGHT)); + case RIGHT: + return new Range<>( + mCrop.left + pixelDistanceToFraction(mCropTouchMargin, + CropBoundary.LEFT), 1f); + } + return null; + } + private void updateListener(MotionEvent event) { if (mCropInteractionListener != null && (isVertical(mCurrentDraggingBoundary))) { float boundaryPosition = getBoundaryPosition(mCurrentDraggingBoundary); @@ -371,6 +399,8 @@ public class CropView extends View { private static final int TOP_HANDLE_ID = 1; private static final int BOTTOM_HANDLE_ID = 2; + private static final int LEFT_HANDLE_ID = 3; + private static final int RIGHT_HANDLE_ID = 4; AccessibilityHelper() { super(CropView.this); @@ -384,62 +414,125 @@ public class CropView extends View { if (Math.abs(y - fractionToVerticalPixels(mCrop.bottom)) < mCropTouchMargin) { return BOTTOM_HANDLE_ID; } - return ExploreByTouchHelper.INVALID_ID; + if (y > fractionToVerticalPixels(mCrop.top) + && y < fractionToVerticalPixels(mCrop.bottom)) { + if (Math.abs(x - fractionToHorizontalPixels(mCrop.left)) < mCropTouchMargin) { + return LEFT_HANDLE_ID; + } + if (Math.abs(x - fractionToHorizontalPixels(mCrop.right)) < mCropTouchMargin) { + return RIGHT_HANDLE_ID; + } + } + + return ExploreByTouchHelper.HOST_ID; } @Override - protected void getVisibleVirtualViews(IntArray virtualViewIds) { + protected void getVisibleVirtualViews(List virtualViewIds) { + // Add views in traversal order virtualViewIds.add(TOP_HANDLE_ID); + virtualViewIds.add(LEFT_HANDLE_ID); + virtualViewIds.add(RIGHT_HANDLE_ID); virtualViewIds.add(BOTTOM_HANDLE_ID); } @Override protected void onPopulateEventForVirtualView(int virtualViewId, AccessibilityEvent event) { - switch (virtualViewId) { - case TOP_HANDLE_ID: - event.setContentDescription( - getResources().getString(R.string.screenshot_top_boundary)); - break; - case BOTTOM_HANDLE_ID: - event.setContentDescription( - getResources().getString(R.string.screenshot_bottom_boundary)); - break; - } + CropBoundary boundary = viewIdToBoundary(virtualViewId); + event.setContentDescription(getBoundaryContentDescription(boundary)); } @Override protected void onPopulateNodeForVirtualView(int virtualViewId, - AccessibilityNodeInfo node) { - switch (virtualViewId) { - case TOP_HANDLE_ID: - node.setContentDescription( - getResources().getString(R.string.screenshot_top_boundary)); - setNodePositions(mCrop.top, node); - break; - case BOTTOM_HANDLE_ID: - node.setContentDescription( - getResources().getString(R.string.screenshot_bottom_boundary)); - setNodePositions(mCrop.bottom, node); - break; - } + AccessibilityNodeInfoCompat node) { + CropBoundary boundary = viewIdToBoundary(virtualViewId); + node.setContentDescription(getBoundaryContentDescription(boundary)); + setNodePosition(getNodeRect(boundary), node); - // TODO: need to figure out the full set of actions to support here. - node.addAction( - AccessibilityNodeInfo.AccessibilityAction.ACTION_CLICK); - node.setClickable(true); - node.setFocusable(true); + // Intentionally set the class name to SeekBar so that TalkBack uses volume control to + // scroll. + node.setClassName(SeekBar.class.getName()); + node.addAction(AccessibilityNodeInfoCompat.ACTION_SCROLL_FORWARD); + node.addAction(AccessibilityNodeInfoCompat.ACTION_SCROLL_BACKWARD); } @Override protected boolean onPerformActionForVirtualView( int virtualViewId, int action, Bundle arguments) { - return false; + if (action != AccessibilityNodeInfo.ACTION_SCROLL_FORWARD + && action != AccessibilityNodeInfo.ACTION_SCROLL_BACKWARD) { + return false; + } + CropBoundary boundary = viewIdToBoundary(virtualViewId); + float delta = pixelDistanceToFraction(mCropTouchMargin, boundary); + if (action == AccessibilityNodeInfo.ACTION_SCROLL_FORWARD) { + delta = -delta; + } + setBoundaryPosition(boundary, delta + getBoundaryPosition(boundary)); + invalidateVirtualView(virtualViewId); + sendEventForVirtualView(virtualViewId, AccessibilityEvent.TYPE_VIEW_SELECTED); + return true; } - private void setNodePositions(float fraction, AccessibilityNodeInfo node) { - int pixels = fractionToVerticalPixels(fraction); - Rect rect = new Rect(0, (int) (pixels - mCropTouchMargin), - getWidth(), (int) (pixels + mCropTouchMargin)); + private CharSequence getBoundaryContentDescription(CropBoundary boundary) { + int template; + switch (boundary) { + case TOP: + template = R.string.screenshot_top_boundary_pct; + break; + case BOTTOM: + template = R.string.screenshot_bottom_boundary_pct; + break; + case LEFT: + template = R.string.screenshot_left_boundary_pct; + break; + case RIGHT: + template = R.string.screenshot_right_boundary_pct; + break; + default: + return ""; + } + + return getResources().getString(template, + Math.round(getBoundaryPosition(boundary) * 100)); + } + + private CropBoundary viewIdToBoundary(int viewId) { + switch (viewId) { + case TOP_HANDLE_ID: + return CropBoundary.TOP; + case BOTTOM_HANDLE_ID: + return CropBoundary.BOTTOM; + case LEFT_HANDLE_ID: + return CropBoundary.LEFT; + case RIGHT_HANDLE_ID: + return CropBoundary.RIGHT; + } + return CropBoundary.NONE; + } + + private Rect getNodeRect(CropBoundary boundary) { + Rect rect; + if (isVertical(boundary)) { + int pixels = fractionToVerticalPixels(getBoundaryPosition(boundary)); + rect = new Rect(0, (int) (pixels - mCropTouchMargin), + getWidth(), (int) (pixels + mCropTouchMargin)); + // Top boundary can sometimes go beyond the view, shift it down to compensate so + // the area is big enough. + if (rect.top < 0) { + rect.offset(0, -rect.top); + } + } else { + int pixels = fractionToHorizontalPixels(getBoundaryPosition(boundary)); + rect = new Rect((int) (pixels - mCropTouchMargin), + (int) (fractionToVerticalPixels(mCrop.top) + mCropTouchMargin), + (int) (pixels + mCropTouchMargin), + (int) (fractionToVerticalPixels(mCrop.bottom) - mCropTouchMargin)); + } + return rect; + } + + private void setNodePosition(Rect rect, AccessibilityNodeInfoCompat node) { node.setBoundsInParent(rect); int[] pos = new int[2]; getLocationOnScreen(pos); From 90ee05903dbfbfbfc919eed53ef62f3e93b1d412 Mon Sep 17 00:00:00 2001 From: Matt Casey Date: Fri, 9 Apr 2021 13:54:29 -0400 Subject: [PATCH 2/2] Prevent CropView from getting confused by multiple pointers. Not really implementing full multitouch, but ensuring that letting up on the first finger doesn't just continue the event with another one. Also refactor some of the MagnifierView's event listening to decouple it from CropView a bit more. Bug: 183240525 Test: Touch one edge with one finger, then put another one down and lift the first, ensure that movement doesn't continue with second finger. Change-Id: Ia9f0293a914fb85f80a746f1093a3b23b23eab73 --- .../android/systemui/screenshot/CropView.java | 86 ++++++++++++++----- .../systemui/screenshot/MagnifierView.java | 81 ++++++++--------- 2 files changed, 105 insertions(+), 62 deletions(-) diff --git a/packages/SystemUI/src/com/android/systemui/screenshot/CropView.java b/packages/SystemUI/src/com/android/systemui/screenshot/CropView.java index bbce7b09e9b67..9e11451afa064 100644 --- a/packages/SystemUI/src/com/android/systemui/screenshot/CropView.java +++ b/packages/SystemUI/src/com/android/systemui/screenshot/CropView.java @@ -71,6 +71,7 @@ public class CropView extends View { private int mImageWidth; private CropBoundary mCurrentDraggingBoundary = CropBoundary.NONE; + private int mActivePointerId; // The starting value of mCurrentDraggingBoundary's crop, used to compute touch deltas. private float mMovementStartValue; private float mStartingY; // y coordinate of ACTION_DOWN @@ -138,35 +139,60 @@ public class CropView extends View { public boolean onTouchEvent(MotionEvent event) { int topPx = fractionToVerticalPixels(mCrop.top); int bottomPx = fractionToVerticalPixels(mCrop.bottom); - switch (event.getAction()) { + switch (event.getActionMasked()) { case MotionEvent.ACTION_DOWN: mCurrentDraggingBoundary = nearestBoundary(event, topPx, bottomPx, fractionToHorizontalPixels(mCrop.left), fractionToHorizontalPixels(mCrop.right)); if (mCurrentDraggingBoundary != CropBoundary.NONE) { + mActivePointerId = event.getPointerId(0); mStartingY = event.getY(); mStartingX = event.getX(); mMovementStartValue = getBoundaryPosition(mCurrentDraggingBoundary); - updateListener(event); + updateListener(MotionEvent.ACTION_DOWN, event.getX()); mMotionRange = getAllowedValues(mCurrentDraggingBoundary); } return true; case MotionEvent.ACTION_MOVE: if (mCurrentDraggingBoundary != CropBoundary.NONE) { - float deltaPx = isVertical(mCurrentDraggingBoundary) ? event.getY() - mStartingY - : event.getX() - mStartingX; - float delta = pixelDistanceToFraction((int) deltaPx, mCurrentDraggingBoundary); - setBoundaryPosition(mCurrentDraggingBoundary, - mMotionRange.clamp(mMovementStartValue + delta)); - updateListener(event); - invalidate(); + int pointerIndex = event.findPointerIndex(mActivePointerId); + if (pointerIndex >= 0) { + // Original pointer still active, do the move. + float deltaPx = isVertical(mCurrentDraggingBoundary) + ? event.getY(pointerIndex) - mStartingY + : event.getX(pointerIndex) - mStartingX; + float delta = pixelDistanceToFraction((int) deltaPx, + mCurrentDraggingBoundary); + setBoundaryPosition(mCurrentDraggingBoundary, + mMotionRange.clamp(mMovementStartValue + delta)); + updateListener(MotionEvent.ACTION_MOVE, event.getX(pointerIndex)); + invalidate(); + } return true; } + break; + case MotionEvent.ACTION_POINTER_DOWN: + if (mActivePointerId == event.getPointerId(event.getActionIndex()) + && mCurrentDraggingBoundary != CropBoundary.NONE) { + updateListener(MotionEvent.ACTION_DOWN, event.getX(event.getActionIndex())); + return true; + } + break; + case MotionEvent.ACTION_POINTER_UP: + if (mActivePointerId == event.getPointerId(event.getActionIndex()) + && mCurrentDraggingBoundary != CropBoundary.NONE) { + updateListener(MotionEvent.ACTION_UP, event.getX(event.getActionIndex())); + return true; + } + break; case MotionEvent.ACTION_CANCEL: case MotionEvent.ACTION_UP: - if (mCurrentDraggingBoundary != CropBoundary.NONE) { - updateListener(event); + if (mCurrentDraggingBoundary != CropBoundary.NONE + && mActivePointerId == event.getPointerId(mActivePointerId)) { + updateListener(MotionEvent.ACTION_UP, event.getX(0)); + return true; } + break; } return super.onTouchEvent(event); } @@ -308,12 +334,29 @@ public class CropView extends View { return null; } - private void updateListener(MotionEvent event) { - if (mCropInteractionListener != null && (isVertical(mCurrentDraggingBoundary))) { + /** + * @param action either ACTION_DOWN, ACTION_UP or ACTION_MOVE. + * @param x coordinate of the relevant pointer. + */ + private void updateListener(int action, float x) { + if (mCropInteractionListener != null && isVertical(mCurrentDraggingBoundary)) { float boundaryPosition = getBoundaryPosition(mCurrentDraggingBoundary); - mCropInteractionListener.onCropMotionEvent(event, mCurrentDraggingBoundary, - boundaryPosition, fractionToVerticalPixels(boundaryPosition), - (mCrop.left + mCrop.right) / 2); + switch (action) { + case MotionEvent.ACTION_DOWN: + mCropInteractionListener.onCropDragStarted(mCurrentDraggingBoundary, + boundaryPosition, fractionToVerticalPixels(boundaryPosition), + (mCrop.left + mCrop.right) / 2, x); + break; + case MotionEvent.ACTION_MOVE: + mCropInteractionListener.onCropDragMoved(mCurrentDraggingBoundary, + boundaryPosition, fractionToVerticalPixels(boundaryPosition), + (mCrop.left + mCrop.right) / 2, x); + break; + case MotionEvent.ACTION_UP: + mCropInteractionListener.onCropDragComplete(); + break; + + } } } @@ -545,12 +588,11 @@ public class CropView extends View { * Listen for crop motion events and state. */ public interface CropInteractionListener { - /** - * Called whenever CropView has a MotionEvent that can impact the position of the crop - * boundaries. - */ - void onCropMotionEvent(MotionEvent event, CropBoundary boundary, float boundaryPosition, - int boundaryPositionPx, float horizontalCenter); + void onCropDragStarted(CropBoundary boundary, float boundaryPosition, + int boundaryPositionPx, float horizontalCenter, float x); + void onCropDragMoved(CropBoundary boundary, float boundaryPosition, + int boundaryPositionPx, float horizontalCenter, float x); + void onCropDragComplete(); } static class SavedState extends BaseSavedState { diff --git a/packages/SystemUI/src/com/android/systemui/screenshot/MagnifierView.java b/packages/SystemUI/src/com/android/systemui/screenshot/MagnifierView.java index 08cd91ccada54..34b40f79836b8 100644 --- a/packages/SystemUI/src/com/android/systemui/screenshot/MagnifierView.java +++ b/packages/SystemUI/src/com/android/systemui/screenshot/MagnifierView.java @@ -28,7 +28,6 @@ import android.graphics.Path; import android.graphics.Rect; import android.graphics.drawable.Drawable; import android.util.AttributeSet; -import android.view.MotionEvent; import android.view.View; import android.view.ViewPropertyAnimator; @@ -148,49 +147,51 @@ public class MagnifierView extends View implements CropView.CropInteractionListe } @Override - public void onCropMotionEvent(MotionEvent event, CropView.CropBoundary boundary, - float cropPosition, int cropPositionPx, float horizontalCenter) { + public void onCropDragStarted(CropView.CropBoundary boundary, float boundaryPosition, + int boundaryPositionPx, float horizontalCenter, float x) { mCropBoundary = boundary; mLastCenter = horizontalCenter; - boolean touchOnRight = event.getX() > getParentWidth() / 2; + boolean touchOnRight = x > getParentWidth() / 2; float translateXTarget = touchOnRight ? 0 : getParentWidth() - getWidth(); - switch (event.getAction()) { - case MotionEvent.ACTION_DOWN: - mLastCropPosition = cropPosition; - setTranslationY(cropPositionPx - getHeight() / 2); - setPivotX(getWidth() / 2); - setPivotY(getHeight() / 2); - setScaleX(0.2f); - setScaleY(0.2f); - setAlpha(0f); - setTranslationX((getParentWidth() - getWidth()) / 2); - setVisibility(View.VISIBLE); - mTranslationAnimator = - animate().alpha(1f).translationX(translateXTarget).scaleX(1f).scaleY(1f); - mTranslationAnimator.setListener(mTranslationAnimatorListener); - mTranslationAnimator.start(); - break; - case MotionEvent.ACTION_MOVE: - // The touch is near the middle if it's within 10% of the center point. - // We don't want to animate horizontally if the touch is near the middle. - boolean nearMiddle = Math.abs(event.getX() - getParentWidth() / 2) - < getParentWidth() / 10f; - boolean viewOnLeft = getTranslationX() < (getParentWidth() - getWidth()) / 2; - if (!nearMiddle && viewOnLeft != touchOnRight && mTranslationAnimator == null) { - mTranslationAnimator = animate().translationX(translateXTarget); - mTranslationAnimator.setListener(mTranslationAnimatorListener); - mTranslationAnimator.start(); - } - mLastCropPosition = cropPosition; - setTranslationY(cropPositionPx - getHeight() / 2); - invalidate(); - break; - case MotionEvent.ACTION_CANCEL: - case MotionEvent.ACTION_UP: - animate().alpha(0).translationX((getParentWidth() - getWidth()) / 2).scaleX(0.2f) - .scaleY(0.2f).withEndAction(() -> setVisibility(View.INVISIBLE)).start(); - break; + mLastCropPosition = boundaryPosition; + setTranslationY(boundaryPositionPx - getHeight() / 2); + setPivotX(getWidth() / 2); + setPivotY(getHeight() / 2); + setScaleX(0.2f); + setScaleY(0.2f); + setAlpha(0f); + setTranslationX((getParentWidth() - getWidth()) / 2); + setVisibility(View.VISIBLE); + mTranslationAnimator = + animate().alpha(1f).translationX(translateXTarget).scaleX(1f).scaleY(1f); + mTranslationAnimator.setListener(mTranslationAnimatorListener); + mTranslationAnimator.start(); + } + + @Override + public void onCropDragMoved(CropView.CropBoundary boundary, float boundaryPosition, + int boundaryPositionPx, float horizontalCenter, float x) { + boolean touchOnRight = x > getParentWidth() / 2; + float translateXTarget = touchOnRight ? 0 : getParentWidth() - getWidth(); + // The touch is near the middle if it's within 10% of the center point. + // We don't want to animate horizontally if the touch is near the middle. + boolean nearMiddle = Math.abs(x - getParentWidth() / 2) + < getParentWidth() / 10f; + boolean viewOnLeft = getTranslationX() < (getParentWidth() - getWidth()) / 2; + if (!nearMiddle && viewOnLeft != touchOnRight && mTranslationAnimator == null) { + mTranslationAnimator = animate().translationX(translateXTarget); + mTranslationAnimator.setListener(mTranslationAnimatorListener); + mTranslationAnimator.start(); } + mLastCropPosition = boundaryPosition; + setTranslationY(boundaryPositionPx - getHeight() / 2); + invalidate(); + } + + @Override + public void onCropDragComplete() { + animate().alpha(0).translationX((getParentWidth() - getWidth()) / 2).scaleX(0.2f) + .scaleY(0.2f).withEndAction(() -> setVisibility(View.INVISIBLE)).start(); } private Path generateCheckerboard() {