From da0bd7b4121de16b03243e629773a6d1b0094cf9 Mon Sep 17 00:00:00 2001 From: Matthew DeVore Date: Mon, 10 Feb 2025 17:55:00 +0000 Subject: [PATCH] Detect when user wanted to drag vs. click In order to avoid sending a new topology to DisplayManager unnecessarily, which could cause some disruptive visual effect, don't do anything if the drag was both (a) brief and (b) did nto deviate from start position. Flag: com.android.settings.flags.display_topology_pane_in_display_list Test: DisplayTopologyPanePreferenceTest Test: manual - quickly drag as few pixels as possible and verify the block moved back after drag, with no scale change Bug: b/352648432 Change-Id: I29ffb51c54c9dbac970149cffd86a8027f0a42f5 --- .../display/DisplayTopology.kt | 88 +++++++++++------- .../display/DisplayTopologyPreferenceTest.kt | 93 +++++++++++++++++++ 2 files changed, 149 insertions(+), 32 deletions(-) diff --git a/src/com/android/settings/connecteddevice/display/DisplayTopology.kt b/src/com/android/settings/connecteddevice/display/DisplayTopology.kt index 949d5bb418d..9cb79eed2ef 100644 --- a/src/com/android/settings/connecteddevice/display/DisplayTopology.kt +++ b/src/com/android/settings/connecteddevice/display/DisplayTopology.kt @@ -224,6 +224,20 @@ class DisplayTopologyPreference(context : Context) @VisibleForTesting var injector : Injector + /** + * How many physical pixels to move in pane coordinates (Pythagorean distance) before a drag is + * considered non-trivial and intentional. + * + * This value is computed on-demand so that the injector can be changed at any time. + */ + @VisibleForTesting val accidentalDragDistancePx + get() = DisplayTopology.dpToPx(4f, injector.densityDpi) + + /** + * How long before until a tap is considered a drag regardless of distance moved. + */ + @VisibleForTesting val accidentalDragTimeLimitMs = 800L + /** * This is needed to prevent a repopulation of the pane causing another * relayout and vice-versa ad infinitum. @@ -295,15 +309,19 @@ class DisplayTopologyPreference(context : Context) open val wallpaper: Bitmap? get() = WallpaperManager.getInstance(context).bitmap - open val densityDpi: Int - get() { - val info = DisplayInfo() - return if (context.display.getDisplayInfo(info)) { - info.logicalDensityDpi - } else { - DisplayMetrics.DENSITY_DEFAULT - } + /** + * This density is the density of the current display (showing the topology pane). It is + * necessary to use this density here because the topology pane coordinates are in physical + * pixels, and the display coordinates are in density-independent pixels. + */ + open val densityDpi: Int by lazy { + val info = DisplayInfo() + if (context.display.getDisplayInfo(info)) { + info.logicalDensityDpi + } else { + DisplayMetrics.DENSITY_DEFAULT } + } open fun registerTopologyListener(listener: Consumer) { displayManager.registerTopologyListener(context.mainExecutor, listener) @@ -323,23 +341,29 @@ class DisplayTopologyPreference(context : Context) val positions: List>) /** - * Holds information about the current drag operation. + * Holds information about the current drag operation. The initial rawX, rawY values of the + * cursor are recorded in order to detect whether the drag was a substantial drag or likely + * accidental. + * * @param stationaryDisps ID and position of displays that are not moving * @param display View that is currently being dragged * @param displayId ID of display being dragged * @param displayWidth width of display being dragged in actual (not View) coordinates * @param displayHeight height of display being dragged in actual (not View) coordinates - * @param dragOffsetX difference between event rawX coordinate and X of the display in the pane - * @param dragOffsetY difference between event rawY coordinate and Y of the display in the pane - * @param didMove true if we have detected the user intentionally wanted to drag rather than - * just click + * @param initialBlockX block's X coordinate upon touch down event + * @param initialBlockY block's Y coordinate upon touch down event + * @param initialTouchX rawX value of the touch down event + * @param initialTouchY rawY value of the touch down event + * @param startTimeMs time when tap down occurred, needed to detect the user intentionally + * wanted to drag rather than just click */ private data class BlockDrag( val stationaryDisps : List>, val display: DisplayBlock, val displayId: Int, val displayWidth: Float, val displayHeight: Float, - val dragOffsetX: Float, val dragOffsetY: Float, - var didMove: Boolean = false) + val initialBlockX: Float, val initialBlockY: Float, + val initialTouchX: Float, val initialTouchY: Float, + val startTimeMs: Long) private var mTopologyInfo : TopologyInfo? = null private var mDrag : BlockDrag? = null @@ -394,14 +418,10 @@ class DisplayTopologyPreference(context : Context) recycleableBlocks.add(mPaneContent.getChildAt(i) as DisplayBlock) } - // This density is the density of the current display (showing the topology pane). It is - // necessary to use this density here because the topology pane coordinates are in physical - // pixels, and the display coordinates are in density-independent pixels. - val dpi = injector.densityDpi val scaling = TopologyScale( mPaneContent.width, - minEdgeLength = DisplayTopology.dpToPx(60f, dpi), - maxEdgeLength = DisplayTopology.dpToPx(256f, dpi), + minEdgeLength = DisplayTopology.dpToPx(60f, injector.densityDpi), + maxEdgeLength = DisplayTopology.dpToPx(256f, injector.densityDpi), newBounds.map { it.second }.toList()) mPaneHolder.layoutParams.let { val newHeight = scaling.paneHeight.toInt() @@ -431,7 +451,7 @@ class DisplayTopologyPreference(context : Context) when (ev.actionMasked) { MotionEvent.ACTION_DOWN -> onBlockTouchDown(id, pos, block, ev) MotionEvent.ACTION_MOVE -> onBlockTouchMove(ev) - MotionEvent.ACTION_UP -> onBlockTouchUp() + MotionEvent.ACTION_UP -> onBlockTouchUp(ev) else -> false } } @@ -462,7 +482,10 @@ class DisplayTopologyPreference(context : Context) // moving, and the raw coordinates are relative to the screen. mDrag = BlockDrag( stationaryDisps.toList(), block, displayId, displayPos.width(), displayPos.height(), - ev.rawX - block.x, ev.rawY - block.y) + initialBlockX = block.x, initialBlockY = block.y, + initialTouchX = ev.rawX, initialTouchY = ev.rawY, + startTimeMs = ev.eventTime, + ) // Prevents a container of this view from intercepting the touch events in the case the // pointer moves outside of the display block or the pane. @@ -474,30 +497,31 @@ class DisplayTopologyPreference(context : Context) val drag = mDrag ?: return false val topology = mTopologyInfo ?: return false val dispDragCoor = topology.scaling.paneToDisplayCoor( - ev.rawX - drag.dragOffsetX, ev.rawY - drag.dragOffsetY) + ev.rawX - drag.initialTouchX + drag.initialBlockX, + ev.rawY - drag.initialTouchY + drag.initialBlockY) val dispDragRect = RectF( dispDragCoor.x, dispDragCoor.y, dispDragCoor.x + drag.displayWidth, dispDragCoor.y + drag.displayHeight) val snapRect = clampPosition(drag.stationaryDisps.map { it.second }, dispDragRect) drag.display.place(topology.scaling.displayToPaneCoor(snapRect.left, snapRect.top)) - drag.didMove = true return true } - private fun onBlockTouchUp(): Boolean { + private fun onBlockTouchUp(ev: MotionEvent): Boolean { val drag = mDrag ?: return false val topology = mTopologyInfo ?: return false mPaneContent.requestDisallowInterceptTouchEvent(false) drag.display.setHighlighted(false) - mDrag = null - if (!drag.didMove) { - // If no move event occurred, ignore the drag completely. - // TODO(b/352648432): Responding to a single move event no matter how small may be too - // sensitive. It is easy to slide by a small amount just by force of pressing down the - // mouse button. Keep an eye on this. + val netPxDragged = Math.hypot( + (drag.initialBlockX - drag.display.x).toDouble(), + (drag.initialBlockY - drag.display.y).toDouble()) + val timeDownMs = ev.eventTime - drag.startTimeMs + if (netPxDragged < accidentalDragDistancePx && timeDownMs < accidentalDragTimeLimitMs) { + drag.display.x = drag.initialBlockX + drag.display.y = drag.initialBlockY return true } diff --git a/tests/robotests/src/com/android/settings/connecteddevice/display/DisplayTopologyPreferenceTest.kt b/tests/robotests/src/com/android/settings/connecteddevice/display/DisplayTopologyPreferenceTest.kt index 8b7f64ccd55..f4c1bcad50a 100644 --- a/tests/robotests/src/com/android/settings/connecteddevice/display/DisplayTopologyPreferenceTest.kt +++ b/tests/robotests/src/com/android/settings/connecteddevice/display/DisplayTopologyPreferenceTest.kt @@ -419,4 +419,97 @@ class DisplayTopologyPreferenceTest { .build()) assertThat(leftBlock.background).isEqualTo(leftBlock.mUnselectedImage) } + + fun dragBlockWithOneMoveEvent( + block: DisplayBlock, startTimeMs: Long, endTimeMs: Long, xDiff: Float, yDiff: Float) { + block.dispatchTouchEvent(MotionEventBuilder.newBuilder() + .setAction(MotionEvent.ACTION_DOWN) + .setPointer(0f, 0f) + .setEventTime(startTimeMs) + .build()) + block.dispatchTouchEvent(MotionEventBuilder.newBuilder() + .setAction(MotionEvent.ACTION_MOVE) + .setPointer(xDiff, yDiff) + .setEventTime((startTimeMs + endTimeMs) / 2) + .build()) + block.dispatchTouchEvent(MotionEventBuilder.newBuilder() + .setAction(MotionEvent.ACTION_UP) + .setPointer(xDiff, yDiff) + .setEventTime(endTimeMs) + .build()) + } + + @Test + fun accidentalDrag_LittleAndBriefEnoughToBeAccidental() { + val (leftBlock, _) = setupTwoDisplays(POSITION_LEFT, childOffset = 42f) + val startTime = 424242L + val startX = leftBlock.x + val startY = leftBlock.y + + preference.mTimesRefreshedBlocks = 0 + dragBlockWithOneMoveEvent( + leftBlock, startTime, + endTimeMs = startTime + preference.accidentalDragTimeLimitMs - 10, + xDiff = preference.accidentalDragDistancePx - 1f, yDiff = 0f, + ) + assertThat(leftBlock.x).isEqualTo(startX) + assertThat(preference.mTimesRefreshedBlocks).isEqualTo(0) + + dragBlockWithOneMoveEvent( + leftBlock, startTime, + endTimeMs = startTime + preference.accidentalDragTimeLimitMs - 10, + xDiff = 0f, yDiff = preference.accidentalDragDistancePx - 1f, + ) + assertThat(leftBlock.y).isEqualTo(startY) + assertThat(preference.mTimesRefreshedBlocks).isEqualTo(0) + } + + @Test + fun accidentalDrag_TooFarToBeAccidentalXAxis() { + val (topBlock, _) = setupTwoDisplays(POSITION_TOP, childOffset = -42f) + val startTime = 88888L + val startX = topBlock.x + + preference.mTimesRefreshedBlocks = 0 + dragBlockWithOneMoveEvent( + topBlock, startTime, + endTimeMs = startTime + preference.accidentalDragTimeLimitMs - 10, + xDiff = preference.accidentalDragDistancePx + 1f, yDiff = 0f, + ) + assertThat(preference.mTimesRefreshedBlocks).isEqualTo(1) + assertThat(topBlock.x).isNotEqualTo(startX) + } + + @Test + fun accidentalDrag_TooFarToBeAccidentalYAxis() { + val (leftBlock, _) = setupTwoDisplays(POSITION_LEFT, childOffset = 42f) + val startTime = 88888L + val startY = leftBlock.y + + preference.mTimesRefreshedBlocks = 0 + dragBlockWithOneMoveEvent( + leftBlock, startTime, + endTimeMs = startTime + preference.accidentalDragTimeLimitMs - 10, + xDiff = 0f, yDiff = preference.accidentalDragDistancePx + 1f, + ) + assertThat(leftBlock.y).isNotEqualTo(startY) + assertThat(preference.mTimesRefreshedBlocks).isEqualTo(1) + } + + @Test + fun accidentalDrag_TooSlowToBeAccidental() { + val (topBlock, _) = setupTwoDisplays(POSITION_TOP, childOffset = -42f) + val startTime = 88888L + val startX = topBlock.x + val startY = topBlock.y + + preference.mTimesRefreshedBlocks = 0 + dragBlockWithOneMoveEvent( + topBlock, startTime, + endTimeMs = startTime + preference.accidentalDragTimeLimitMs + 10, + xDiff = preference.accidentalDragDistancePx - 1f, yDiff = 0f, + ) + assertThat(topBlock.x).isNotEqualTo(startX) + assertThat(preference.mTimesRefreshedBlocks).isEqualTo(1) + } }