From f40976cd248f47e4e883db315ad5e837bd78a1c2 Mon Sep 17 00:00:00 2001 From: Xin Chen Date: Mon, 25 Apr 2022 14:11:11 -0700 Subject: [PATCH] Refactor findTargetPathAndCoordinatesForTouch to improve perf of event delivery Summary: Refactor of TouchTargetHelper.findTargetPathAndCoordinatesForTouch to avoid unnecessary lookup of views during the dispatching of Hover Events changelog: [internal] internal Reviewed By: lunaleaps, mdvacca Differential Revision: D32296003 fbshipit-source-id: 93834c37331ad5d75645a5665a1c8c3d965765fb --- .../react/uimanager/JSPointerDispatcher.java | 76 +++++++++++-------- .../react/uimanager/TouchTargetHelper.java | 69 ++++++++++++++--- 2 files changed, 103 insertions(+), 42 deletions(-) diff --git a/ReactAndroid/src/main/java/com/facebook/react/uimanager/JSPointerDispatcher.java b/ReactAndroid/src/main/java/com/facebook/react/uimanager/JSPointerDispatcher.java index 71e06856e03..5fdb514ef48 100644 --- a/ReactAndroid/src/main/java/com/facebook/react/uimanager/JSPointerDispatcher.java +++ b/ReactAndroid/src/main/java/com/facebook/react/uimanager/JSPointerDispatcher.java @@ -13,6 +13,7 @@ import android.view.ViewGroup; import com.facebook.common.logging.FLog; import com.facebook.infer.annotation.Assertions; import com.facebook.react.common.ReactConstants; +import com.facebook.react.uimanager.TouchTargetHelper.ViewTarget; import com.facebook.react.uimanager.events.EventDispatcher; import com.facebook.react.uimanager.events.PointerEvent; import com.facebook.react.uimanager.events.PointerEventHelper; @@ -40,7 +41,7 @@ public class JSPointerDispatcher { // Set globally for hover interactions, referenced for coalescing hover events private long mHoverInteractionKey = TouchEvent.UNSET; - private List mLastHitPath = Collections.emptyList(); + private List mLastHitPath = Collections.emptyList(); private final float[] mLastEventCoordinates = new float[2]; public JSPointerDispatcher(ViewGroup viewGroup) { @@ -56,7 +57,7 @@ public class JSPointerDispatcher { return; } - List hitPath = + List hitPath = TouchTargetHelper.findTargetPathAndCoordinatesForTouch( motionEvent.getX(), motionEvent.getY(), mRootViewGroup, mTargetCoordinates); dispatchCancelEvent(hitPath, motionEvent, eventDispatcher); @@ -74,11 +75,14 @@ public class JSPointerDispatcher { int surfaceId = UIManagerHelper.getSurfaceId(mRootViewGroup); int action = motionEvent.getActionMasked(); - List hitPath = + List hitPath = TouchTargetHelper.findTargetPathAndCoordinatesForTouch( motionEvent.getX(), motionEvent.getY(), mRootViewGroup, mTargetCoordinates); - int targetTag = hitPath.get(0); + if (hitPath.isEmpty()) { + return; + } + int targetTag = hitPath.get(0).getViewId(); if (supportsHover) { if (action == MotionEvent.ACTION_HOVER_MOVE) { @@ -105,7 +109,7 @@ public class JSPointerDispatcher { if (!supportsHover) { // Enter root -> child for (int i = hitPath.size(); i-- > 0; ) { - int tag = hitPath.get(i); + int tag = hitPath.get(i).getViewId(); eventDispatcher.dispatchEvent( PointerEvent.obtain(PointerEventHelper.POINTER_ENTER, surfaceId, tag, motionEvent)); } @@ -161,7 +165,7 @@ public class JSPointerDispatcher { if (!supportsHover) { // Leave child -> root for (int i = 0; i < hitPath.size(); i++) { - int tag = hitPath.get(i); + int tag = hitPath.get(i).getViewId(); eventDispatcher.dispatchEvent( PointerEvent.obtain(PointerEventHelper.POINTER_LEAVE, surfaceId, tag, motionEvent)); } @@ -195,7 +199,7 @@ public class JSPointerDispatcher { MotionEvent motionEvent, EventDispatcher eventDispatcher, int surfaceId, - List hitPath) { + List hitPath) { int action = motionEvent.getActionMasked(); if (action != MotionEvent.ACTION_HOVER_MOVE) { @@ -222,13 +226,17 @@ public class JSPointerDispatcher { // If child is handling, eliminate target tags under handling child if (mChildHandlingNativeGesture > 0) { - int index = hitPath.indexOf(mChildHandlingNativeGesture); - if (index > 0) { - hitPath.subList(0, index).clear(); + int index = 0; + for (ViewTarget viewTarget : hitPath) { + if (viewTarget.getViewId() == mChildHandlingNativeGesture) { + hitPath.subList(0, index).clear(); + break; + } + index++; } } - int targetTag = hitPath.size() > 0 ? hitPath.get(0) : -1; + int targetTag = hitPath.isEmpty() ? -1 : hitPath.get(0).getViewId(); // If targetTag is empty, we should bail? if (targetTag == -1) { return; @@ -252,26 +260,31 @@ public class JSPointerDispatcher { // If something has changed in either enter/exit, let's start a new coalescing key mTouchEventCoalescingKeyHelper.incrementCoalescingKey(mHoverInteractionKey); - List enterTargetTags = hitPath.subList(0, hitPath.size() - firstDivergentIndex); - if (enterTargetTags.size() > 0) { + List enterViewTargets = hitPath.subList(0, hitPath.size() - firstDivergentIndex); + if (enterViewTargets.size() > 0) { // root -> child - for (int i = enterTargetTags.size(); i-- > 0; ) { - int enterTargetTag = enterTargetTags.get(i); + for (int i = enterViewTargets.size(); i-- > 0; ) { eventDispatcher.dispatchEvent( PointerEvent.obtain( - PointerEventHelper.POINTER_ENTER, surfaceId, enterTargetTag, motionEvent)); + PointerEventHelper.POINTER_ENTER, + surfaceId, + enterViewTargets.get(i).getViewId(), + motionEvent)); } } // Fire all relevant exit events - List exitTargetTags = + List exitViewTargets = mLastHitPath.subList(0, mLastHitPath.size() - firstDivergentIndex); - if (exitTargetTags.size() > 0) { + if (exitViewTargets.size() > 0) { // child -> root - for (Integer exitTargetTag : exitTargetTags) { + for (ViewTarget exitViewTarget : exitViewTargets) { eventDispatcher.dispatchEvent( PointerEvent.obtain( - PointerEventHelper.POINTER_LEAVE, surfaceId, exitTargetTag, motionEvent)); + PointerEventHelper.POINTER_LEAVE, + surfaceId, + exitViewTarget.getViewId(), + motionEvent)); } } } @@ -287,7 +300,7 @@ public class JSPointerDispatcher { } private void dispatchCancelEvent( - List hitPath, MotionEvent motionEvent, EventDispatcher eventDispatcher) { + List hitPath, MotionEvent motionEvent, EventDispatcher eventDispatcher) { // This means the gesture has already ended, via some other CANCEL or UP event. This is not // expected to happen very often as it would mean some child View has decided to intercept the // touch stream and start a native gesture only upon receiving the UP/CANCEL event. @@ -297,16 +310,19 @@ public class JSPointerDispatcher { "Expected to not have already sent a cancel for this gesture"); int surfaceId = UIManagerHelper.getSurfaceId(mRootViewGroup); - int targetTag = hitPath.get(0); - // Question: Does cancel fire on all in hit path? - Assertions.assertNotNull(eventDispatcher) - .dispatchEvent( - PointerEvent.obtain( - PointerEventHelper.POINTER_CANCEL, surfaceId, targetTag, motionEvent)); + if (!hitPath.isEmpty()) { + int targetTag = hitPath.get(0).getViewId(); + // Question: Does cancel fire on all in hit path? + Assertions.assertNotNull(eventDispatcher) + .dispatchEvent( + PointerEvent.obtain( + PointerEventHelper.POINTER_CANCEL, surfaceId, targetTag, motionEvent)); - for (int tag : hitPath) { - eventDispatcher.dispatchEvent( - PointerEvent.obtain(PointerEventHelper.POINTER_LEAVE, surfaceId, tag, motionEvent)); + for (ViewTarget viewTarget : hitPath) { + eventDispatcher.dispatchEvent( + PointerEvent.obtain( + PointerEventHelper.POINTER_LEAVE, surfaceId, viewTarget.getViewId(), motionEvent)); + } } mTouchEventCoalescingKeyHelper.removeCoalescingKey(mDownStartTime); diff --git a/ReactAndroid/src/main/java/com/facebook/react/uimanager/TouchTargetHelper.java b/ReactAndroid/src/main/java/com/facebook/react/uimanager/TouchTargetHelper.java index bfac2387e2c..07f97bdcc0d 100644 --- a/ReactAndroid/src/main/java/com/facebook/react/uimanager/TouchTargetHelper.java +++ b/ReactAndroid/src/main/java/com/facebook/react/uimanager/TouchTargetHelper.java @@ -25,6 +25,7 @@ import com.facebook.react.uimanager.common.ViewUtil; import java.util.ArrayList; import java.util.EnumSet; import java.util.List; +import java.util.Objects; /** * Class responsible for identifying which react view should handle a given {@link MotionEvent}. It @@ -109,11 +110,12 @@ public class TouchTargetHelper { * @param eventY the Y screen coordinate of the touch location * @param viewGroup the container view to traverse * @param viewCoords an out parameter that will return the X,Y value in the target view - * @return If a target was found, returns a path through the view tree of all react tags that are - * a container for the touch target, ordered from target to root (last element) + * @return If a target was found, returns a {@link Lis} containing the path through + * the view tree of all react tags and views that are a container for the touch target, + * ordered from target to root (last element) */ @SuppressLint("ResourceType") - public static List findTargetPathAndCoordinatesForTouch( + public static List findTargetPathAndCoordinatesForTouch( float eventX, float eventY, ViewGroup viewGroup, float[] viewCoords) { UiThreadUtil.assertOnUiThread(); @@ -121,7 +123,7 @@ public class TouchTargetHelper { viewCoords[0] = eventX; viewCoords[1] = eventY; - List pathAccumulator = new ArrayList<>(); + List pathAccumulator = new ArrayList<>(); View targetView = findTouchTargetViewWithPointerEvents(viewCoords, viewGroup, pathAccumulator); if (targetView != null) { @@ -140,7 +142,7 @@ public class TouchTargetHelper { int targetTag = getTouchTargetForView(reactTargetView, eventX, eventY); if (targetTag != reactTargetView.getId()) { - pathAccumulator.add(0, targetTag); + pathAccumulator.add(0, new ViewTarget(targetTag, (View) null)); } } @@ -178,7 +180,7 @@ public class TouchTargetHelper { float[] eventCoords, View view, EnumSet allowReturnTouchTargetTypes, - List pathAccumulator) { + List pathAccumulator) { // We prefer returning a child, so we check for a child that can handle the touch first if (allowReturnTouchTargetTypes.contains(TouchTargetReturnType.CHILD) && view instanceof ViewGroup) { @@ -303,7 +305,7 @@ public class TouchTargetHelper { * its descendants are the touch target. */ private static @Nullable View findTouchTargetViewWithPointerEvents( - float eventCoords[], View view, @Nullable List pathAccumulator) { + float eventCoords[], View view, @Nullable List pathAccumulator) { PointerEvents pointerEvents = view instanceof ReactPointerEventsView ? ((ReactPointerEventsView) view).getPointerEvents() @@ -330,7 +332,7 @@ public class TouchTargetHelper { findTouchTargetView( eventCoords, view, EnumSet.of(TouchTargetReturnType.SELF), pathAccumulator); if (targetView != null && pathAccumulator != null) { - pathAccumulator.add(view.getId()); + pathAccumulator.add(new ViewTarget(view.getId(), view)); } return targetView; @@ -341,7 +343,7 @@ public class TouchTargetHelper { eventCoords, view, EnumSet.of(TouchTargetReturnType.CHILD), pathAccumulator); if (targetView != null) { if (pathAccumulator != null) { - pathAccumulator.add(view.getId()); + pathAccumulator.add(new ViewTarget(view.getId(), view)); } return targetView; } @@ -358,7 +360,7 @@ public class TouchTargetHelper { // make sure we exclude the View itself because of the PointerEvents.BOX_NONE if (reactTag != view.getId()) { if (pathAccumulator != null) { - pathAccumulator.add(view.getId()); + pathAccumulator.add(new ViewTarget(view.getId(), view)); } return view; } @@ -372,7 +374,7 @@ public class TouchTargetHelper { && isTouchPointInView(eventCoords[0], eventCoords[1], view) && ((ReactCompoundViewGroup) view).interceptsTouchEvent(eventCoords[0], eventCoords[1])) { if (pathAccumulator != null) { - pathAccumulator.add(view.getId()); + pathAccumulator.add(new ViewTarget(view.getId(), view)); } return view; } @@ -384,7 +386,7 @@ public class TouchTargetHelper { EnumSet.of(TouchTargetReturnType.SELF, TouchTargetReturnType.CHILD), pathAccumulator); if (result != null && pathAccumulator != null) { - pathAccumulator.add(view.getId()); + pathAccumulator.add(new ViewTarget(view.getId(), view)); } return result; @@ -402,4 +404,47 @@ public class TouchTargetHelper { } return targetView.getId(); } + + public static class ViewTarget { + private final int mViewId; + private final @Nullable View mView; + + private ViewTarget(int viewId, @Nullable View view) { + mViewId = viewId; + mView = view; + } + + public int getViewId() { + return mViewId; + } + + @Nullable + public View getView() { + return mView; + } + + @Override + public boolean equals(Object o) { + // If the object is compared with itself then return true + if (o == this) { + return true; + } + + // Check if o is an instance of ViewTarget. Note that "null instanceof ViewTarget" also + // returns false. + if (!(o instanceof ViewTarget)) { + return false; + } + + ViewTarget other = (ViewTarget) o; + // We only need to compare view id, as we assume the same view id will always map to the same + // view. TargetView is not mutable so this should be safe. + return other.getViewId() == mViewId; + } + + @Override + public int hashCode() { + return Objects.hashCode(mViewId); + } + } }