From bdd33e3b59d95df0ab6e80fa59556a29266c357f Mon Sep 17 00:00:00 2001 From: Bryce Lee Date: Tue, 6 Dec 2022 16:37:39 -0800 Subject: [PATCH] Do not specify root view in TouchInsetManager. TouchInsetManager helps limit the touch targets for a surface to a given set of child views. Previously, a root view was specified at construction, from which the attached surface was retrieved from. Since this information is associated with each tracked view, providing this information upfront is not necessary. This changelist updates TouchInsetManager to retrieve the attached surface from the views rather than at construction. The logic has been updated to support multiple root surfaces, allowing the same TouchInsetManager to be used across surfaces. Test: atest TouchInsetManagertest Bug: 261781069 Change-Id: Ief02e781fd3b2beb1373f05192690f664cacb6c1 --- .../dreams/dagger/DreamOverlayModule.java | 5 +- .../systemui/touch/TouchInsetManager.java | 152 +++++++++++------- .../systemui/touch/TouchInsetManagerTest.java | 93 +++++++---- 3 files changed, 157 insertions(+), 93 deletions(-) diff --git a/packages/SystemUI/src/com/android/systemui/dreams/dagger/DreamOverlayModule.java b/packages/SystemUI/src/com/android/systemui/dreams/dagger/DreamOverlayModule.java index 67e25713987f8..4e0f86c8b146f 100644 --- a/packages/SystemUI/src/com/android/systemui/dreams/dagger/DreamOverlayModule.java +++ b/packages/SystemUI/src/com/android/systemui/dreams/dagger/DreamOverlayModule.java @@ -109,9 +109,8 @@ public abstract class DreamOverlayModule { /** */ @Provides @DreamOverlayComponent.DreamOverlayScope - public static TouchInsetManager providesTouchInsetManager(@Main Executor executor, - DreamOverlayContainerView view) { - return new TouchInsetManager(executor, view); + public static TouchInsetManager providesTouchInsetManager(@Main Executor executor) { + return new TouchInsetManager(executor); } /** */ diff --git a/packages/SystemUI/src/com/android/systemui/touch/TouchInsetManager.java b/packages/SystemUI/src/com/android/systemui/touch/TouchInsetManager.java index 3d07491283c5d..8450c1e6e9d08 100644 --- a/packages/SystemUI/src/com/android/systemui/touch/TouchInsetManager.java +++ b/packages/SystemUI/src/com/android/systemui/touch/TouchInsetManager.java @@ -18,8 +18,9 @@ package com.android.systemui.touch; import android.graphics.Rect; import android.graphics.Region; +import android.util.Log; +import android.view.AttachedSurfaceControl; import android.view.View; -import android.view.ViewRootImpl; import androidx.concurrent.futures.CallbackToFutureAdapter; @@ -34,25 +35,37 @@ import java.util.concurrent.Executor; * is useful for passing through touch events for all but select areas. */ public class TouchInsetManager { + private static final String TAG = "TouchInsetManager"; /** * {@link TouchInsetSession} provides an individualized session with the * {@link TouchInsetManager}, linking any action to the client. */ public static class TouchInsetSession { private final TouchInsetManager mManager; - private final HashSet mTrackedViews; private final Executor mExecutor; private final View.OnLayoutChangeListener mOnLayoutChangeListener = (v, left, top, right, bottom, oldLeft, oldTop, oldRight, oldBottom) - -> updateTouchRegion(); + -> updateTouchRegions(); + + private final View.OnAttachStateChangeListener mAttachListener = + new View.OnAttachStateChangeListener() { + @Override + public void onViewAttachedToWindow(View v) { + updateTouchRegions(); + } + + @Override + public void onViewDetachedFromWindow(View v) { + updateTouchRegions(); + } + }; /** * Default constructor * @param manager The parent {@link TouchInsetManager} which will be affected by actions on * this session. - * @param rootView The parent of views that will be tracked. * @param executor An executor for marshalling operations. */ TouchInsetSession(TouchInsetManager manager, Executor executor) { @@ -68,8 +81,9 @@ public class TouchInsetManager { public void addViewToTracking(View view) { mExecutor.execute(() -> { mTrackedViews.add(view); + view.addOnAttachStateChangeListener(mAttachListener); view.addOnLayoutChangeListener(mOnLayoutChangeListener); - updateTouchRegion(); + updateTouchRegions(); }); } @@ -81,22 +95,30 @@ public class TouchInsetManager { mExecutor.execute(() -> { mTrackedViews.remove(view); view.removeOnLayoutChangeListener(mOnLayoutChangeListener); - updateTouchRegion(); + view.removeOnAttachStateChangeListener(mAttachListener); + updateTouchRegions(); }); } - private void updateTouchRegion() { - final Region cumulativeRegion = Region.obtain(); + private void updateTouchRegions() { + mExecutor.execute(() -> { + final HashMap affectedSurfaces = new HashMap<>(); + mTrackedViews.stream().forEach(view -> { + if (!view.isAttachedToWindow()) { + return; + } - mTrackedViews.stream().forEach(view -> { - final Rect boundaries = new Rect(); - view.getBoundsOnScreen(boundaries); - cumulativeRegion.op(boundaries, Region.Op.UNION); + final AttachedSurfaceControl surface = view.getRootSurfaceControl(); + + if (!affectedSurfaces.containsKey(surface)) { + affectedSurfaces.put(surface, Region.obtain()); + } + final Rect boundaries = new Rect(); + view.getBoundsOnScreen(boundaries); + affectedSurfaces.get(surface).op(boundaries, Region.Op.UNION); + }); + mManager.setTouchRegions(this, affectedSurfaces); }); - - mManager.setTouchRegion(this, cumulativeRegion); - - cumulativeRegion.recycle(); } /** @@ -110,32 +132,17 @@ public class TouchInsetManager { } } - private final HashMap mDefinedRegions = new HashMap<>(); + private final HashMap> + mSessionRegions = new HashMap<>(); + private final HashMap mLastAffectedSurfaces = new HashMap(); private final Executor mExecutor; - private final View mRootView; - - private final View.OnAttachStateChangeListener mAttachListener = - new View.OnAttachStateChangeListener() { - @Override - public void onViewAttachedToWindow(View v) { - updateTouchInset(); - } - - @Override - public void onViewDetachedFromWindow(View v) { - } - }; /** * Default constructor. * @param executor An {@link Executor} to marshal all operations on. - * @param rootView The root {@link View} for all views in sessions. */ - public TouchInsetManager(Executor executor, View rootView) { + public TouchInsetManager(Executor executor) { mExecutor = executor; - mRootView = rootView; - mRootView.addOnAttachStateChangeListener(mAttachListener); - } /** @@ -151,47 +158,68 @@ public class TouchInsetManager { public ListenableFuture checkWithinTouchRegion(int x, int y) { return CallbackToFutureAdapter.getFuture(completer -> { mExecutor.execute(() -> completer.set( - mDefinedRegions.values().stream().anyMatch(region -> region.contains(x, y)))); + mLastAffectedSurfaces.values().stream().anyMatch( + region -> region.contains(x, y)))); return "DreamOverlayTouchMonitor::checkWithinTouchRegion"; }); } - private void updateTouchInset() { - final ViewRootImpl viewRootImpl = mRootView.getViewRootImpl(); + private void updateTouchInsets() { + // Get affected + final HashMap affectedSurfaces = new HashMap<>(); + mSessionRegions.values().stream().forEach(regionMapping -> { + regionMapping.entrySet().stream().forEach(entry -> { + final AttachedSurfaceControl surface = entry.getKey(); + if (!affectedSurfaces.containsKey(surface)) { + affectedSurfaces.put(surface, Region.obtain()); + } - if (viewRootImpl == null) { + affectedSurfaces.get(surface).op(entry.getValue(), Region.Op.UNION); + }); + }); + + affectedSurfaces.entrySet().stream().forEach(entry -> { + entry.getKey().setTouchableRegion(entry.getValue()); + }); + + mLastAffectedSurfaces.entrySet().forEach(entry -> { + final AttachedSurfaceControl surface = entry.getKey(); + if (!affectedSurfaces.containsKey(surface)) { + surface.setTouchableRegion(null); + } + entry.getValue().recycle(); + }); + + mLastAffectedSurfaces.clear(); + mLastAffectedSurfaces.putAll(affectedSurfaces); + } + + protected void setTouchRegions(TouchInsetSession session, + HashMap regions) { + mExecutor.execute(() -> { + recycleRegions(session); + mSessionRegions.put(session, regions); + updateTouchInsets(); + }); + } + + private void recycleRegions(TouchInsetSession session) { + if (!mSessionRegions.containsKey(session)) { + Log.w(TAG, "Removing a session with no regions:" + session); return; } - final Region aggregateRegion = Region.obtain(); - - for (Region region : mDefinedRegions.values()) { - aggregateRegion.op(region, Region.Op.UNION); + for (Region region : mSessionRegions.get(session).values()) { + region.recycle(); } - - viewRootImpl.setTouchableRegion(aggregateRegion); - - aggregateRegion.recycle(); - } - - protected void setTouchRegion(TouchInsetSession session, Region region) { - final Region introducedRegion = Region.obtain(region); - mExecutor.execute(() -> { - mDefinedRegions.put(session, introducedRegion); - updateTouchInset(); - }); } private void clearRegion(TouchInsetSession session) { mExecutor.execute(() -> { - final Region storedRegion = mDefinedRegions.remove(session); - - if (storedRegion != null) { - storedRegion.recycle(); - } - - updateTouchInset(); + recycleRegions(session); + mSessionRegions.remove(session); + updateTouchInsets(); }); } } diff --git a/packages/SystemUI/tests/src/com/android/systemui/touch/TouchInsetManagerTest.java b/packages/SystemUI/tests/src/com/android/systemui/touch/TouchInsetManagerTest.java index 14b9bfb1393f9..a7072225baa74 100644 --- a/packages/SystemUI/tests/src/com/android/systemui/touch/TouchInsetManagerTest.java +++ b/packages/SystemUI/tests/src/com/android/systemui/touch/TouchInsetManagerTest.java @@ -26,8 +26,8 @@ import static org.mockito.Mockito.when; import android.graphics.Rect; import android.graphics.Region; import android.testing.AndroidTestingRunner; +import android.view.AttachedSurfaceControl; import android.view.View; -import android.view.ViewRootImpl; import androidx.test.filters.SmallTest; @@ -47,41 +47,78 @@ import org.mockito.MockitoAnnotations; @RunWith(AndroidTestingRunner.class) public class TouchInsetManagerTest extends SysuiTestCase { @Mock - private View mRootView; - - @Mock - private ViewRootImpl mRootViewImpl; + private AttachedSurfaceControl mAttachedSurfaceControl; private FakeExecutor mFakeExecutor = new FakeExecutor(new FakeSystemClock()); @Before public void setup() { MockitoAnnotations.initMocks(this); - when(mRootView.getViewRootImpl()).thenReturn(mRootViewImpl); } @Test - public void testRootViewOnAttachedHandling() { + public void testViewOnAttachedHandling() { // Create inset manager - final TouchInsetManager insetManager = new TouchInsetManager(mFakeExecutor, - mRootView); + final TouchInsetManager insetManager = new TouchInsetManager(mFakeExecutor); final ArgumentCaptor listener = ArgumentCaptor.forClass(View.OnAttachStateChangeListener.class); + final View view = createView(new Rect(0, 0, 0, 0)); + when(view.isAttachedToWindow()).thenReturn(false); + + // Create session + final TouchInsetManager.TouchInsetSession session = insetManager.createSession(); + session.addViewToTracking(view); + + mFakeExecutor.runAllReady(); // Ensure manager has registered to listen to attached state of root view. - verify(mRootView).addOnAttachStateChangeListener(listener.capture()); + verify(view).addOnAttachStateChangeListener(listener.capture()); + + clearInvocations(mAttachedSurfaceControl); + when(view.isAttachedToWindow()).thenReturn(true); // Trigger attachment and verify touchable region is set. - listener.getValue().onViewAttachedToWindow(mRootView); - verify(mRootViewImpl).setTouchableRegion(any()); + listener.getValue().onViewAttachedToWindow(view); + + mFakeExecutor.runAllReady(); + + verify(mAttachedSurfaceControl).setTouchableRegion(any()); + } + + @Test + public void testViewOnDetachedHandling() { + // Create inset manager + final TouchInsetManager insetManager = new TouchInsetManager(mFakeExecutor); + + final ArgumentCaptor listener = + ArgumentCaptor.forClass(View.OnAttachStateChangeListener.class); + final View view = createView(new Rect(0, 0, 0, 0)); + when(view.isAttachedToWindow()).thenReturn(true); + + // Create session + final TouchInsetManager.TouchInsetSession session = insetManager.createSession(); + session.addViewToTracking(view); + + mFakeExecutor.runAllReady(); + // Ensure manager has registered to listen to attached state of root view. + verify(view).addOnAttachStateChangeListener(listener.capture()); + + clearInvocations(mAttachedSurfaceControl); + when(view.isAttachedToWindow()).thenReturn(false); + + // Trigger detachment and verify touchable region is set. + listener.getValue().onViewDetachedFromWindow(view); + + mFakeExecutor.runAllReady(); + + verify(mAttachedSurfaceControl).setTouchableRegion(any()); } @Test public void testInsetRegionPropagation() { // Create inset manager - final TouchInsetManager insetManager = new TouchInsetManager(mFakeExecutor, - mRootView); + final TouchInsetManager insetManager = new TouchInsetManager(mFakeExecutor); // Create session final TouchInsetManager.TouchInsetSession session = insetManager.createSession(); @@ -95,14 +132,13 @@ public class TouchInsetManagerTest extends SysuiTestCase { // Check to see if view was properly accounted for. final Region expectedRegion = Region.obtain(); expectedRegion.op(rect, Region.Op.UNION); - verify(mRootViewImpl).setTouchableRegion(eq(expectedRegion)); + verify(mAttachedSurfaceControl).setTouchableRegion(eq(expectedRegion)); } @Test public void testMultipleRegions() { // Create inset manager - final TouchInsetManager insetManager = new TouchInsetManager(mFakeExecutor, - mRootView); + final TouchInsetManager insetManager = new TouchInsetManager(mFakeExecutor); // Create session final TouchInsetManager.TouchInsetSession session = insetManager.createSession(); @@ -112,7 +148,7 @@ public class TouchInsetManagerTest extends SysuiTestCase { session.addViewToTracking(createView(firstBounds)); mFakeExecutor.runAllReady(); - clearInvocations(mRootViewImpl); + clearInvocations(mAttachedSurfaceControl); // Create second session final TouchInsetManager.TouchInsetSession secondSession = insetManager.createSession(); @@ -128,27 +164,26 @@ public class TouchInsetManagerTest extends SysuiTestCase { final Region expectedRegion = Region.obtain(); expectedRegion.op(firstBounds, Region.Op.UNION); expectedRegion.op(secondBounds, Region.Op.UNION); - verify(mRootViewImpl).setTouchableRegion(eq(expectedRegion)); + verify(mAttachedSurfaceControl).setTouchableRegion(eq(expectedRegion)); } - clearInvocations(mRootViewImpl); + clearInvocations(mAttachedSurfaceControl); // clear first session, ensure second session is still reflected. session.clear(); mFakeExecutor.runAllReady(); { final Region expectedRegion = Region.obtain(); - expectedRegion.op(firstBounds, Region.Op.UNION); - verify(mRootViewImpl).setTouchableRegion(eq(expectedRegion)); + expectedRegion.op(secondBounds, Region.Op.UNION); + verify(mAttachedSurfaceControl).setTouchableRegion(eq(expectedRegion)); } } @Test public void testMultipleViews() { // Create inset manager - final TouchInsetManager insetManager = new TouchInsetManager(mFakeExecutor, - mRootView); + final TouchInsetManager insetManager = new TouchInsetManager(mFakeExecutor); // Create session final TouchInsetManager.TouchInsetSession session = insetManager.createSession(); @@ -159,7 +194,7 @@ public class TouchInsetManagerTest extends SysuiTestCase { // only capture second invocation. mFakeExecutor.runAllReady(); - clearInvocations(mRootViewImpl); + clearInvocations(mAttachedSurfaceControl); // Add a second view to the session final Rect secondViewBounds = new Rect(4, 4, 9, 10); @@ -173,20 +208,20 @@ public class TouchInsetManagerTest extends SysuiTestCase { final Region expectedRegion = Region.obtain(); expectedRegion.op(firstViewBounds, Region.Op.UNION); expectedRegion.op(secondViewBounds, Region.Op.UNION); - verify(mRootViewImpl).setTouchableRegion(eq(expectedRegion)); + verify(mAttachedSurfaceControl).setTouchableRegion(eq(expectedRegion)); } // Remove second view. session.removeViewFromTracking(secondView); - clearInvocations(mRootViewImpl); + clearInvocations(mAttachedSurfaceControl); mFakeExecutor.runAllReady(); // Ensure first view still reflected in touch region. { final Region expectedRegion = Region.obtain(); expectedRegion.op(firstViewBounds, Region.Op.UNION); - verify(mRootViewImpl).setTouchableRegion(eq(expectedRegion)); + verify(mAttachedSurfaceControl).setTouchableRegion(eq(expectedRegion)); } } @@ -197,6 +232,8 @@ public class TouchInsetManagerTest extends SysuiTestCase { ((Rect) invocation.getArgument(0)).set(rect); return null; }).when(view).getBoundsOnScreen(any()); + when(view.isAttachedToWindow()).thenReturn(true); + when(view.getRootSurfaceControl()).thenReturn(mAttachedSurfaceControl); return view; }