diff options
5 files changed, 268 insertions, 41 deletions
diff --git a/core/java/android/view/ScrollCaptureSearchResults.java b/core/java/android/view/ScrollCaptureSearchResults.java index 3469b9dc7103..b1ce949f9919 100644 --- a/core/java/android/view/ScrollCaptureSearchResults.java +++ b/core/java/android/view/ScrollCaptureSearchResults.java @@ -16,10 +16,16 @@ package android.view; +import static android.view.flags.Flags.scrollCaptureTargetZOrderFix; + +import static java.util.Comparator.comparing; import static java.util.Objects.requireNonNull; +import static java.util.Objects.requireNonNullElse; import android.annotation.NonNull; +import android.annotation.Nullable; import android.annotation.UiThread; +import android.graphics.Point; import android.graphics.Rect; import android.os.CancellationSignal; import android.util.IndentingPrintWriter; @@ -113,7 +119,9 @@ public final class ScrollCaptureSearchResults { private void signalComplete() { mComplete = true; - mTargets.sort(PRIORITY_ORDER); + if (!scrollCaptureTargetZOrderFix()) { + mTargets.sort(PRIORITY_ORDER); + } if (mOnCompleteListener != null) { mOnCompleteListener.run(); mOnCompleteListener = null; @@ -125,14 +133,73 @@ public final class ScrollCaptureSearchResults { return new ArrayList<>(mTargets); } + private Rect getScrollBoundsInWindow(@Nullable ScrollCaptureTarget target) { + if (target == null || target.getScrollBounds() == null) { + return new Rect(); + } + Rect windowRect = new Rect(target.getScrollBounds()); + Point windowPosition = target.getPositionInWindow(); + windowRect.offset(windowPosition.x, windowPosition.y); + return windowRect; + } + /** * Get the top ranked result out of all completed requests. * * @return the top ranked result */ + @Nullable public ScrollCaptureTarget getTopResult() { - ScrollCaptureTarget target = mTargets.isEmpty() ? null : mTargets.get(0); - return target != null && target.getScrollBounds() != null ? target : null; + if (!scrollCaptureTargetZOrderFix()) { + ScrollCaptureTarget target = mTargets.isEmpty() ? null : mTargets.get(0); + return target != null && target.getScrollBounds() != null ? target : null; + } + List<ScrollCaptureTarget> filtered = new ArrayList<>(); + + mTargets.removeIf(a -> nullOrEmpty(a.getScrollBounds())); + + // Remove scroll targets obscured or covered by other scrolling views. + nextTarget: + for (int i = 0; i < mTargets.size(); i++) { + ScrollCaptureTarget current = mTargets.get(i); + + View currentView = current.getContainingView(); + + // Nested scroll containers: + // Check if the next view is a child of the current. If so, skip the current. + if (i + 1 < mTargets.size()) { + ScrollCaptureTarget next = mTargets.get(i + 1); + View nextView = next.getContainingView(); + // Honor explicit include hint on parent as escape hatch (unless both have it) + if (isDescendant(currentView, nextView) + && (!hasIncludeHint(currentView) || hasIncludeHint(nextView))) { + continue; + } + } + + // Check if any views will be drawn partially or fully over this one. + for (int j = i + 1; j < mTargets.size(); j++) { + ScrollCaptureTarget above = mTargets.get(j); + if (Rect.intersects(getScrollBoundsInWindow(current), + getScrollBoundsInWindow(above))) { + continue nextTarget; + } + } + + filtered.add(current); + } + + // natural order, false->true + Comparator<ScrollCaptureTarget> byIncludeHintPresence = comparing( + ScrollCaptureSearchResults::hasIncludeHint); + + // natural order, smallest->largest area + Comparator<ScrollCaptureTarget> byArea = comparing( + target -> area(requireNonNullElse(target.getScrollBounds(), new Rect()))); + + // The top result is the last one (with include hint if present, then by largest area) + filtered.sort(byIncludeHintPresence.thenComparing(byArea)); + return filtered.isEmpty() ? null : filtered.getLast(); } private class SearchRequest implements Consumer<Rect> { @@ -226,6 +293,10 @@ public final class ScrollCaptureSearchResults { return r == null || r.isEmpty(); } + private static boolean hasIncludeHint(ScrollCaptureTarget target) { + return hasIncludeHint(target.getContainingView()); + } + private static boolean hasIncludeHint(View view) { return (view.getScrollCaptureHint() & View.SCROLL_CAPTURE_HINT_INCLUDE) != 0; } diff --git a/core/java/android/view/ViewGroup.java b/core/java/android/view/ViewGroup.java index 4a9916c007c4..949b667f0b7a 100644 --- a/core/java/android/view/ViewGroup.java +++ b/core/java/android/view/ViewGroup.java @@ -20,6 +20,7 @@ import static android.view.WindowInsetsAnimation.Callback.DISPATCH_MODE_CONTINUE import static android.view.WindowInsetsAnimation.Callback.DISPATCH_MODE_STOP; import static android.view.flags.Flags.FLAG_TOOLKIT_VIEWGROUP_SET_REQUESTED_FRAME_RATE_API; import static android.view.flags.Flags.toolkitViewgroupSetRequestedFrameRateApi; +import static android.view.flags.Flags.scrollCaptureTargetZOrderFix; import android.animation.LayoutTransition; import android.annotation.CallSuper; @@ -7657,6 +7658,11 @@ public abstract class ViewGroup extends View implements ViewParent, ViewManager @NonNull Rect localVisibleRect, @NonNull Point windowOffset, @NonNull Consumer<ScrollCaptureTarget> targets) { + // Only visible views can be captured. + if (getVisibility() != View.VISIBLE) { + return; + } + if (getClipToPadding() && !localVisibleRect.intersect(mPaddingLeft, mPaddingTop, (mRight - mLeft) - mPaddingRight, (mBottom - mTop) - mPaddingBottom)) { return; @@ -7665,19 +7671,39 @@ public abstract class ViewGroup extends View implements ViewParent, ViewManager // Dispatch to self first. super.dispatchScrollCaptureSearch(localVisibleRect, windowOffset, targets); + final int childrenCount = mChildrenCount; + if (childrenCount == 0) { + return; + } + // Skip children if descendants excluded. if ((getScrollCaptureHint() & SCROLL_CAPTURE_HINT_EXCLUDE_DESCENDANTS) != 0) { return; } - final Rect tmpRect = getTempRect(); - final int childCount = getChildCount(); - for (int i = 0; i < childCount; i++) { - View child = getChildAt(i); + + ArrayList<View> preorderedList = null; + boolean customOrder = false; + if (scrollCaptureTargetZOrderFix()) { + preorderedList = buildOrderedChildList(); + customOrder = preorderedList == null && isChildrenDrawingOrderEnabled(); + } + final View[] children = mChildren; + for (int i = 0; i < childrenCount; i++) { + View child; + if (scrollCaptureTargetZOrderFix()) { + // Traverse children in the same order they will be drawn (honors Z if set) + final int childIndex = getAndVerifyPreorderedIndex(childrenCount, i, customOrder); + child = getAndVerifyPreorderedView(preorderedList, children, childIndex); + } else { + child = children[i]; + } + // Only visible views can be captured. if (child.getVisibility() != View.VISIBLE) { continue; } + // Offset the given rectangle (in parent's local coordinates) into child's coordinate // space and clip the result to the child View's bounds, padding and clipRect as needed. // If the resulting rectangle is not empty, the request is forwarded to the child. @@ -7706,6 +7732,9 @@ public abstract class ViewGroup extends View implements ViewParent, ViewManager child.dispatchScrollCaptureSearch(tmpRect, childWindowOffset, targets); } } + if (preorderedList != null) { + preorderedList.clear(); + } } /** diff --git a/core/java/android/view/flags/scroll_capture.aconfig b/core/java/android/view/flags/scroll_capture.aconfig new file mode 100644 index 000000000000..fdf9c1ed8ad2 --- /dev/null +++ b/core/java/android/view/flags/scroll_capture.aconfig @@ -0,0 +1,13 @@ +package: "android.view.flags" +container: "system" + +flag { + name: "scroll_capture_target_z_order_fix" + namespace: "system_ui" + description: "Always prefer targets with higher z-order" + bug: "365969802" + metadata { + purpose: PURPOSE_BUGFIX + } +} + diff --git a/core/tests/coretests/src/android/view/ScrollCaptureSearchResultsTest.java b/core/tests/coretests/src/android/view/ScrollCaptureSearchResultsTest.java index 726ee85dddd5..915ace058121 100644 --- a/core/tests/coretests/src/android/view/ScrollCaptureSearchResultsTest.java +++ b/core/tests/coretests/src/android/view/ScrollCaptureSearchResultsTest.java @@ -16,9 +16,9 @@ package android.view; -import static androidx.test.InstrumentationRegistry.getTargetContext; +import static android.view.flags.Flags.FLAG_SCROLL_CAPTURE_TARGET_Z_ORDER_FIX; -import static com.google.common.truth.Truth.assertThat; +import static androidx.test.InstrumentationRegistry.getTargetContext; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertNull; @@ -32,13 +32,16 @@ import android.graphics.Point; import android.graphics.Rect; import android.os.CancellationSignal; import android.os.SystemClock; +import android.platform.test.annotations.EnableFlags; import android.platform.test.annotations.Presubmit; +import android.platform.test.flag.junit.SetFlagsRule; import androidx.annotation.NonNull; import androidx.test.ext.junit.runners.AndroidJUnit4; import androidx.test.filters.SmallTest; import org.junit.Before; +import org.junit.Rule; import org.junit.Test; import org.junit.runner.RunWith; @@ -56,6 +59,8 @@ import java.util.function.Consumer; @RunWith(AndroidJUnit4.class) public class ScrollCaptureSearchResultsTest { + @Rule + public final SetFlagsRule mSetFlagsRule = new SetFlagsRule(); private static final Rect EMPTY_RECT = new Rect(); private static final String TAG = "Test"; @@ -98,6 +103,45 @@ public class ScrollCaptureSearchResultsTest { assertNull("Expected null due to no valid targets", results.getTopResult()); } + /** + * A scrolling target should be excluded even when larger if it will be drawn over by another + * scrolling target. + */ + @EnableFlags(FLAG_SCROLL_CAPTURE_TARGET_Z_ORDER_FIX) + @Test + public void testCoveredTargetsAreExcluded() { + ScrollCaptureSearchResults results = new ScrollCaptureSearchResults(mDirectExec); + + FakeScrollCaptureCallback callback1 = new FakeScrollCaptureCallback(mDirectExec); + callback1.setScrollBounds(new Rect(0, 0, 200, 200)); // 200 tall + View view1 = new FakeView(getTargetContext(), 0, 0, 200, 200, 1); + ScrollCaptureTarget target1 = createTargetWithView(view1, callback1, + new Rect(0, 0, 200, 200), new Point(0, 0), View.SCROLL_CAPTURE_HINT_AUTO); + + FakeScrollCaptureCallback callback2 = new FakeScrollCaptureCallback(mDirectExec); + callback2.setScrollBounds(new Rect(0, 0, 200, 180)); // 180 tall + View view2 = new FakeView(getTargetContext(), 0, 20, 200, 200, 2); + ScrollCaptureTarget target2 = createTargetWithView(view2, callback2, + new Rect(0, 0, 200, 180), new Point(0, 20), View.SCROLL_CAPTURE_HINT_AUTO); + + // Top z-order but smaller, and non-intersecting. (positioned further Y than the first two) + FakeScrollCaptureCallback callback3 = new FakeScrollCaptureCallback(mDirectExec); + callback3.setScrollBounds(new Rect(0, 0, 50, 50)); + View view3 = new FakeView(getTargetContext(), 75, 250, 125, 300, 3); + ScrollCaptureTarget target3 = createTargetWithView(view3, callback3, + new Rect(0, 0, 50, 50), new Point(75, 250), View.SCROLL_CAPTURE_HINT_AUTO); + + results.addTarget(target1); + results.addTarget(target2); + results.addTarget(target3); + + assertTrue(results.isComplete()); + ScrollCaptureTarget result = results.getTopResult(); + assertSame("Expected the second target because of higher z-Index", target2, result); + assertEquals("result has wrong scroll bounds", + new Rect(0, 0, 200, 180), result.getScrollBounds()); + } + @Test public void testSingleTarget() { ScrollCaptureSearchResults results = new ScrollCaptureSearchResults(mDirectExec); @@ -152,29 +196,29 @@ public class ScrollCaptureSearchResultsTest { // 2 - 10x10 + HINT_INCLUDE FakeScrollCaptureCallback callback2 = new FakeScrollCaptureCallback(mDirectExec); - callback2.setScrollBounds(new Rect(0, 0, 10, 10)); - ViewGroup targetView2 = new FakeView(getTargetContext(), 0, 0, 60, 60, 2); + callback2.setScrollBounds(new Rect(25, 25, 35, 35)); // 10x10 + ViewGroup targetView2 = new FakeView(getTargetContext(), 0, 60, 60, 120, 2); ScrollCaptureTarget target2 = createTargetWithView(targetView2, callback2, new Rect(0, 0, 60, 60), new Point(0, 0), View.SCROLL_CAPTURE_HINT_INCLUDE); // 3 - 20x20 + AUTO FakeScrollCaptureCallback callback3 = new FakeScrollCaptureCallback(mDirectExec); callback3.setScrollBounds(new Rect(0, 0, 20, 20)); - ViewGroup targetView3 = new FakeView(getTargetContext(), 0, 0, 60, 60, 3); + ViewGroup targetView3 = new FakeView(getTargetContext(), 0, 120, 60, 180, 3); ScrollCaptureTarget target3 = createTargetWithView(targetView3, callback3, new Rect(0, 0, 60, 60), new Point(0, 0), View.SCROLL_CAPTURE_HINT_AUTO); // 4 - 30x30 + AUTO FakeScrollCaptureCallback callback4 = new FakeScrollCaptureCallback(mDirectExec); callback4.setScrollBounds(new Rect(0, 0, 10, 10)); - ViewGroup targetView4 = new FakeView(getTargetContext(), 0, 0, 60, 60, 4); + ViewGroup targetView4 = new FakeView(getTargetContext(), 0, 180, 60, 240, 4); ScrollCaptureTarget target4 = createTargetWithView(targetView4, callback4, new Rect(0, 0, 60, 60), new Point(0, 0), View.SCROLL_CAPTURE_HINT_AUTO); // 5 - 10x10 + child of #4 FakeScrollCaptureCallback callback5 = new FakeScrollCaptureCallback(mDirectExec); callback5.setScrollBounds(new Rect(0, 0, 10, 10)); - ViewGroup targetView5 = new FakeView(getTargetContext(), 0, 0, 60, 60, 5); + ViewGroup targetView5 = new FakeView(getTargetContext(), 0, 0, 60, 30, 5); ScrollCaptureTarget target5 = createTargetWithView(targetView5, callback5, new Rect(0, 0, 60, 60), new Point(0, 0), View.SCROLL_CAPTURE_HINT_AUTO); targetView4.addView(targetView5); @@ -182,7 +226,7 @@ public class ScrollCaptureSearchResultsTest { // 6 - 20x20 + child of #4 FakeScrollCaptureCallback callback6 = new FakeScrollCaptureCallback(mDirectExec); callback6.setScrollBounds(new Rect(0, 0, 20, 20)); - ViewGroup targetView6 = new FakeView(getTargetContext(), 0, 0, 60, 60, 6); + ViewGroup targetView6 = new FakeView(getTargetContext(), 0, 30, 30, 60, 6); ScrollCaptureTarget target6 = createTargetWithView(targetView6, callback6, new Rect(0, 0, 60, 60), new Point(0, 0), View.SCROLL_CAPTURE_HINT_AUTO); targetView4.addView(targetView6); @@ -194,20 +238,10 @@ public class ScrollCaptureSearchResultsTest { results.addTarget(target4); results.addTarget(target5); results.addTarget(target6); - assertTrue(results.isComplete()); + assertTrue("results.isComplete()", results.isComplete()); // Verify "top" result - assertEquals(target2, results.getTopResult()); - - // Verify priority ("best" first) - assertThat(results.getTargets()) - .containsExactly( - target2, - target6, - target5, - target4, - target3, - target1); + assertEquals("top result", target2, results.getTopResult()); } /** @@ -291,27 +325,22 @@ public class ScrollCaptureSearchResultsTest { new Rect(1, 2, 3, 4), result.getScrollBounds()); } - private void setupTargetView(View view, Rect localVisibleRect, int scrollCaptureHint) { - view.setScrollCaptureHint(scrollCaptureHint); - view.onVisibilityAggregated(true); - // Treat any offset as padding, outset localVisibleRect on all sides and use this as - // child bounds - Rect bounds = new Rect(localVisibleRect); - bounds.inset(-bounds.left, -bounds.top, bounds.left, bounds.top); - view.layout(bounds.left, bounds.top, bounds.right, bounds.bottom); - view.onVisibilityAggregated(true); - } - private ScrollCaptureTarget createTarget(ScrollCaptureCallback callback, Rect localVisibleRect, Point positionInWindow, int scrollCaptureHint) { - View mockView = new View(getTargetContext()); + Rect bounds = new Rect(localVisibleRect); + // Use localVisibleRect as position, treat left/top offset as padding + bounds.left = 0; + bounds.top = 0; + View mockView = new FakeView(getTargetContext(), bounds.left, bounds.top, bounds.right, + bounds.bottom, View.NO_ID); return createTargetWithView(mockView, callback, localVisibleRect, positionInWindow, scrollCaptureHint); } private ScrollCaptureTarget createTargetWithView(View view, ScrollCaptureCallback callback, Rect localVisibleRect, Point positionInWindow, int scrollCaptureHint) { - setupTargetView(view, localVisibleRect, scrollCaptureHint); + view.setScrollCaptureHint(scrollCaptureHint); + view.onVisibilityAggregated(true); return new ScrollCaptureTarget(view, localVisibleRect, positionInWindow, callback); } @@ -326,6 +355,32 @@ public class ScrollCaptureSearchResultsTest { @Override protected void onLayout(boolean changed, int l, int t, int r, int b) { } + + /** Ignores window attachment state. The standard impl always returns [0,0] if the view is + * not attached. This override allows testing without dealing with AttachInfo. + */ + @Override + public void getLocationInWindow(int[] outLocation) { + outLocation[0] = mLeft; + outLocation[1] = mTop; + ViewParent viewParent = getParent(); + while (viewParent instanceof View) { + final View view = (View) viewParent; + + outLocation[0] -= view.mScrollX; + outLocation[1] -= view.mScrollY; + + // Explicitly do not handle matrix/transforms, not needed for testing + if (!view.hasIdentityMatrix()) { + throw new IllegalStateException("This mock does not handle transforms!"); + } + + outLocation[0] += view.mLeft; + outLocation[1] += view.mTop; + + viewParent = view.mParent; + } + } } static class FakeScrollCaptureCallback implements ScrollCaptureCallback { diff --git a/core/tests/coretests/src/android/view/ViewGroupScrollCaptureTest.java b/core/tests/coretests/src/android/view/ViewGroupScrollCaptureTest.java index 25608c328f95..adf7a720071a 100644 --- a/core/tests/coretests/src/android/view/ViewGroupScrollCaptureTest.java +++ b/core/tests/coretests/src/android/view/ViewGroupScrollCaptureTest.java @@ -16,6 +16,8 @@ package android.view; +import static android.view.flags.Flags.FLAG_SCROLL_CAPTURE_TARGET_Z_ORDER_FIX; + import static androidx.test.platform.app.InstrumentationRegistry.getInstrumentation; import static org.junit.Assert.assertEquals; @@ -30,16 +32,20 @@ import android.content.Context; import android.graphics.Point; import android.graphics.Rect; import android.os.CancellationSignal; +import android.platform.test.annotations.EnableFlags; import android.platform.test.annotations.Presubmit; +import android.platform.test.flag.junit.SetFlagsRule; import androidx.annotation.NonNull; import androidx.test.filters.MediumTest; import androidx.test.filters.SmallTest; +import org.junit.Rule; import org.junit.Test; import org.junit.runner.RunWith; import org.mockito.junit.MockitoJUnitRunner; +import java.util.List; import java.util.concurrent.Executor; import java.util.function.Consumer; @@ -51,6 +57,9 @@ import java.util.function.Consumer; @RunWith(MockitoJUnitRunner.class) public class ViewGroupScrollCaptureTest { + @Rule + public final SetFlagsRule mSetFlagsRule = new SetFlagsRule(); + private static final Executor DIRECT_EXECUTOR = Runnable::run; /** Make sure the hint flags are saved and loaded correctly. */ @@ -239,6 +248,56 @@ public class ViewGroupScrollCaptureTest { assertNull(results.getTopResult()); } + @EnableFlags(FLAG_SCROLL_CAPTURE_TARGET_Z_ORDER_FIX) + @MediumTest + @Test + public void testDispatchScrollCaptureSearch_traversesInDrawingOrder() throws Exception { + final Context context = getInstrumentation().getContext(); + // Uses childDrawingOrder to reverse drawing order of children. + final MockViewGroup viewGroup = new MockViewGroup(context, 0, 0, 200, 200); + + // w=200, h=180, z=10, drawn on top + final MockView view1 = new MockView(context, 0, 20, 200, 200); + TestScrollCaptureCallback callback1 = new TestScrollCaptureCallback(); + view1.setScrollCaptureCallback(callback1); + view1.setZ(10f); + + // w=200, h=200, z=0, drawn first, under view1 + final MockView view2 = new MockView(context, 0, 0, 200, 200); + TestScrollCaptureCallback callback2 = new TestScrollCaptureCallback(); + view2.setScrollCaptureCallback(callback2); + + viewGroup.addView(view1); // test order is dependent on draw order by adding z=10 first + viewGroup.addView(view2); + + Rect localVisibleRect = new Rect(0, 0, 200, 200); + Point windowOffset = new Point(0, 0); + + // Where targets are added + final ScrollCaptureSearchResults results = new ScrollCaptureSearchResults(DIRECT_EXECUTOR); + + viewGroup.dispatchScrollCaptureSearch(localVisibleRect, windowOffset, results::addTarget); + callback1.completeSearchRequest(new Rect(0, 0, 200, 180)); + callback2.completeSearchRequest(new Rect(0, 0, 200, 200)); + assertTrue(results.isComplete()); + + List<ScrollCaptureTarget> targets = results.getTargets(); + List<View> targetViews = + targets.stream().map(ScrollCaptureTarget::getContainingView).toList(); + assertEquals(List.of(view2, view1), targetViews); + } + + static final class ReverseDrawingViewGroup extends MockViewGroup { + ReverseDrawingViewGroup(Context context, int left, int top, int right, int bottom) { + super(context, left, top, right, bottom, View.SCROLL_CAPTURE_HINT_AUTO); + } + + @Override + protected int getChildDrawingOrder(int childCount, int drawingPosition) { + return childCount == 0 ? 0 : childCount - (drawingPosition + 1); + } + } + /** * Test scroll capture search dispatch to child views. * <p> @@ -511,7 +570,7 @@ public class ViewGroupScrollCaptureTest { } }; - public static final class MockViewGroup extends ViewGroup { + public static class MockViewGroup extends ViewGroup { private ScrollCaptureCallback mInternalCallback; private Rect mOnScrollCaptureSearchLastLocalVisibleRect; private Point mOnScrollCaptureSearchLastWindowOffset; |