Merge "Simplify pane sizing and scaling algorithm" into main

src/com/android/settings/connecteddevice/display/DisplayTopology.kt

@@ -67,10 +67,10 @@ fun Float.atLeast(n: Number): Float = max(this, n.toFloat())
  * pane coordinates is necessary when rendering the original topology. Conversion in the other
  * direction, to display coordinates, is necessary for resolve a drag position to display space.
  *
- * The topology pane coordinates are integral and represent the relative position from the upper-
+ * The topology pane coordinates are physical pixels and represent the relative position from the
- * left corner of the pane. It uses a scale optimized for showing all displays with minimal or no
+ * upper-left corner of the pane. It uses a scale optimized for showing all displays with minimal
- * scrolling. The display coordinates are floating point and the origin can be in any position. In
+ * or no scrolling. The display coordinates are floating point and the origin can be in any
- * practice the origin will be the upper-left coordinate of the primary display.
+ * position. In practice the origin will be the upper-left coordinate of the primary display.
  *
  * @param paneWidth width of the pane in view coordinates
  * @param minPaneHeight smallest allowed height of the pane in view coordinates. This will not
@@ -79,14 +79,13 @@ fun Float.atLeast(n: Number): Float = max(this, n.toFloat())
  * @param minEdgeLength the smallest length permitted of a display block. This should be set based
  *                      on accessibility requirements, but also accounting for padding that appears
  *                      around each button.
- * @param maxBlockRatio the highest allowed ratio of block size to display size. For instance, a
+ * @param maxEdgeLength the longest width or height permitted of a display block. This will limit
- *                      value of 0.05 means the block will at most be 1/20 the size of the display
+ *                      the amount of dragging and scrolling the user will need to do to set the
- *                      it represents. This limit may be breached to account for minEdgeLength,
+ *                      arrangement.
- *                      which is considered an a11y requirement.
  * @param displaysPos the absolute topology coordinates for each display in the topology.
  */
 class TopologyScale(
-        paneWidth: Int, minPaneHeight: Float, minEdgeLength: Float, maxBlockRatio: Float,
+        paneWidth: Int, minPaneHeight: Float, minEdgeLength: Float, maxEdgeLength: Float,
         displaysPos: Collection<RectF>) {
     /** Scale of block sizes to real-world display sizes. Should be less than 1. */
     val blockRatio: Float
@@ -104,40 +103,33 @@ class TopologyScale(
         val displayBounds = RectF(
                 Float.MAX_VALUE, Float.MAX_VALUE, Float.MIN_VALUE, Float.MIN_VALUE)
         var smallestDisplayDim = Float.MAX_VALUE
-        var biggestDisplayHeight = Float.MIN_VALUE
+        var biggestDisplayDim = Float.MIN_VALUE
 
         // displayBounds is the smallest rect encompassing all displays, in display space.
         // smallestDisplayDim is the size of the smallest display edge, in display space.
         for (pos in displaysPos) {
             displayBounds.union(pos)
             smallestDisplayDim = minOf(smallestDisplayDim, pos.height(), pos.width())
-            biggestDisplayHeight = max(biggestDisplayHeight, pos.height())
+            biggestDisplayDim = maxOf(biggestDisplayDim, pos.height(), pos.width())
         }
 
-        // Set height according to the width and the aspect ratio of the display bounds limited by
+        // Initialize blockRatio such that there is 20% padding on left and right sides of the
-        // maxBlockRatio. It prevents blocks from being too large, which would make dragging and
+        // display bounds.
-        // dropping awkward.
+        blockRatio = (paneWidth * 0.6 / displayBounds.width()).toFloat()
-        blockRatio = maxBlockRatio
+                // If the `ratio` is set too high because one of the displays will have an edge
-                .atMost(paneWidth * 0.6 / displayBounds.width())
+                // greater than maxEdgeLength(px) long, decrease it such that the largest edge is
-                // If the `ratio` is set too low because one of the displays will have an edge less
+                // that long.
-                // than minEdgeLength(dp) long, increase it such that the smallest edge is that
+                .atMost(maxEdgeLength / biggestDisplayDim)
-                // long.
+                // Also do the opposite of the above, this latter step taking precedence for a11y
+                // requirements.
                 .atLeast(minEdgeLength / smallestDisplayDim)
 
-        // Essentially, we just set the pane height based on the pre-determined pane width and the
+        paneHeight = minPaneHeight
-        // aspect ratio of the display bounds.
+                // A tall pane is likely to result in more scrolling. So we
-        paneHeight = (paneWidth.toFloat() / displayBounds.width() * displayBounds.height())
-                // We may need to increase it slightly to achieve 20% padding above and below the
-                // display bounds - this is where the 0.6 comes from.
-                .atLeast(displayBounds.height() * blockRatio / 0.6)
-
-                // It is easy for the aspect ratio to result in an excessively tall pane, since the
-                // width is pre-determined and may be considerably wider than necessary. So we
                 // prevent the height from growing too large here, by limiting vertical padding to
-                // the size of the tallest display. This improves results for very tall display
+                // 1.5x of the minEdgeLength on each side. This keeps a comfortable amount of
-                // bounds.
+                // padding without it resulting in too much deadspace.
-                .atMost(blockRatio * (displayBounds.height() + biggestDisplayHeight * 2f))
+                .atLeast(blockRatio * displayBounds.height() + minEdgeLength * 3f)
-                .atLeast(minPaneHeight)
 
         // Set originPaneXY (the location of 0,0 in display space in the pane's coordinate system)
         // such that the display bounds rect is centered in the pane.
@@ -414,7 +406,7 @@ class DisplayTopologyPreference(context : Context)
         val scaling = TopologyScale(
                 mPaneContent.width, minPaneHeight = mTopologyInfo?.scaling?.paneHeight ?: 0f,
                 minEdgeLength = DisplayTopology.dpToPx(60f, dpi),
-                maxBlockRatio = DisplayTopology.dpToPx(0.12f, dpi),
+                maxEdgeLength = DisplayTopology.dpToPx(256f, dpi),
                 newBounds.map { it.second }.toList())
         mPaneHolder.layoutParams.let {
             val newHeight = scaling.paneHeight.toInt()

tests/robotests/src/com/android/settings/connecteddevice/display/DisplayTopologyPreferenceTest.kt

@@ -334,7 +334,7 @@ class DisplayTopologyPreferenceTest {
     fun updatedTopologyCancelsDragIfNonTrivialChange() {
         val (leftBlock, _) = setupTwoDisplays(POSITION_LEFT, /* childOffset= */ 42f)
 
-        assertThat(leftBlock.y).isWithin(0.01f).of(142.17f)
+        assertThat(leftBlock.y).isWithin(0.05f).of(143.76f)
 
         leftBlock.dispatchTouchEvent(MotionEventBuilder.newBuilder()
                 .setAction(MotionEvent.ACTION_DOWN)
@@ -344,30 +344,30 @@ class DisplayTopologyPreferenceTest {
                 .setAction(MotionEvent.ACTION_MOVE)
                 .setPointer(0f, 30f)
                 .build())
-        assertThat(leftBlock.y).isWithin(0.01f).of(172.17f)
+        assertThat(leftBlock.y).isWithin(0.05f).of(173.76f)
 
         // Offset is only different by 0.5 dp, so the drag will not cancel.
         injector.topology = twoDisplayTopology(POSITION_LEFT, /* childOffset= */ 41.5f)
         injector.topologyListener!!.accept(injector.topology!!)
 
-        assertThat(leftBlock.y).isWithin(0.01f).of(172.17f)
+        assertThat(leftBlock.y).isWithin(0.05f).of(173.76f)
         // Move block farther downward.
         leftBlock.dispatchTouchEvent(MotionEventBuilder.newBuilder()
                 .setAction(MotionEvent.ACTION_MOVE)
                 .setPointer(0f, 50f)
                 .build())
-        assertThat(leftBlock.y).isWithin(0.01f).of(192.17f)
+        assertThat(leftBlock.y).isWithin(0.05f).of(193.76f)
 
         injector.topology = twoDisplayTopology(POSITION_LEFT, /* childOffset= */ 20f)
         injector.topologyListener!!.accept(injector.topology!!)
 
-        assertThat(leftBlock.y).isWithin(0.01f).of(125.67f)
+        assertThat(leftBlock.y).isWithin(0.05f).of(115.60f)
         // Another move in the opposite direction should not move the left block.
         leftBlock.dispatchTouchEvent(MotionEventBuilder.newBuilder()
                 .setAction(MotionEvent.ACTION_MOVE)
                 .setPointer(0f, -20f)
                 .build())
-        assertThat(leftBlock.y).isWithin(0.01f).of(125.67f)
+        assertThat(leftBlock.y).isWithin(0.05f).of(115.60f)
     }
 
     @Test

tests/robotests/src/com/android/settings/connecteddevice/display/TopologyScaleTest.kt

@@ -19,14 +19,25 @@ package com.android.settings.connecteddevice.display
 import android.graphics.Point
 import android.graphics.PointF
 import android.graphics.RectF
+import kotlin.math.abs
 import org.junit.Assert.assertEquals
 import org.junit.Test
 import org.junit.runner.RunWith
 import org.robolectric.RobolectricTestRunner
 
-fun assertPointF(x: Float, y: Float, delta: Float, actual: PointF) {
+// Assert as many points as possible in a single assert since a tweak can change every assertion
-    assertEquals(x, actual.x, delta)
+// in a small way and we don't want to re-compile and re-test for *every* individual float.
-    assertEquals(y, actual.y, delta)
+fun assertPointF(comparisons: List<Pair<PointF, PointF>>, delta: Float) {
+    val errors = StringBuilder()
+    comparisons.forEachIndexed {i, (a, b) ->
+        if (abs(b.x - a.x) > delta) {
+            errors.append("x value at index $i - ${a.x} != ${b.x}\n")
+        }
+        if (abs(b.y - a.y) > delta) {
+            errors.append("y value at index $i - ${a.y} != ${b.y}\n")
+        }
+    }
+    assertEquals("", errors.toString())
 }
 
 @RunWith(RobolectricTestRunner::class)
@@ -35,28 +46,32 @@ class TopologyScaleTest {
     fun oneDisplay4to3Aspect() {
         val scale = TopologyScale(
                 /* paneWidth= */ 640, minPaneHeight = 0f,
-                minEdgeLength = 48f, maxBlockRatio = 0.05f,
+                minEdgeLength = 48f, maxEdgeLength = 64f,
                 listOf(RectF(0f, 0f, 640f, 480f)))
 
-        // blockRatio is higher than 0.05 in order to make the smallest display edge (480 dp) 48dp
+        // blockRatio is is set in order to make the smallest display edge (480 dp) 48dp
         // in the pane.
         assertEquals(
-                "{TopologyScale blockRatio=0.100000 originPaneXY=288.0,48.0 paneHeight=144.0}",
+                "{TopologyScale blockRatio=0.100000 originPaneXY=288.0,72.0 paneHeight=192.0}",
                 "" + scale)
 
-        assertPointF(352f, 96f, 0.001f, scale.displayToPaneCoor(640f, 480f))
+        assertPointF(
-        assertPointF(320f, 72f, 0.001f, scale.displayToPaneCoor(320f, 240f))
+            listOf(
-        assertPointF(640f, 480f, 0.001f, scale.paneToDisplayCoor(352f, 96f))
+                PointF(352f, 120f) to scale.displayToPaneCoor(640f, 480f),
+                PointF(320f, 96f) to scale.displayToPaneCoor(320f, 240f),
+                PointF(640f, 240f) to scale.paneToDisplayCoor(352f, 96f),
+            ),
+            0.001f)
 
         // Same as original scale but made taller with minPaneHeight.
         // The paneHeight and origin coordinates are changed but the block ratio is the same.
         val taller = TopologyScale(
                 /* paneWidth= */ 640, minPaneHeight = 155.0f,
-                minEdgeLength = 48f, maxBlockRatio = 0.05f,
+                minEdgeLength = 48f, maxEdgeLength = 64f,
                 listOf(RectF(0f, 0f, 640f, 480f)))
 
         assertEquals(
-                "{TopologyScale blockRatio=0.100000 originPaneXY=288.0,53.5 paneHeight=155.0}",
+                "{TopologyScale blockRatio=0.100000 originPaneXY=288.0,72.0 paneHeight=192.0}",
                 "" + taller)
     }
 
@@ -64,41 +79,47 @@ class TopologyScaleTest {
     fun twoUnalignedDisplays() {
         val scale = TopologyScale(
                 /* paneWidth= */ 300, minPaneHeight = 0f,
-                minEdgeLength = 48f, maxBlockRatio = 0.05f,
+                minEdgeLength = 48f, maxEdgeLength = 96f,
                 listOf(RectF(0f, 0f, 1920f, 1200f), RectF(1920f, -300f, 3840f, 900f)))
 
         assertEquals(
-                "{TopologyScale blockRatio=0.046875 originPaneXY=60.0,37.5 paneHeight=117.2}",
+                "{TopologyScale blockRatio=0.046875 originPaneXY=60.0,86.1 paneHeight=214.3}",
                 "" + scale)
 
-        assertPointF(78.75f, 56.25f, 0.001f, scale.displayToPaneCoor(400f, 400f))
+        assertPointF(
-        assertPointF(41.25f, 37.5f, 0.001f, scale.displayToPaneCoor(-400f, 0f))
+            listOf(
-        assertPointF(-384f, 96f, 0.001f, scale.paneToDisplayCoor(42f, 42f))
+                PointF(78.75f, 104.8125f) to scale.displayToPaneCoor(400f, 400f),
+                PointF(41.25f, 86.0625f) to scale.displayToPaneCoor(-400f, 0f),
+                PointF(-384f, -940f) to scale.paneToDisplayCoor(42f, 42f),
+            ), 0.001f)
     }
 
     @Test
     fun twoDisplaysBlockRatioBumpedForGarSizeMinimumHorizontal() {
         val scale = TopologyScale(
                 /* paneWidth= */ 192, minPaneHeight = 0f,
-                minEdgeLength = 48f, maxBlockRatio = 0.05f,
+                minEdgeLength = 48f, maxEdgeLength = 64f,
                 listOf(RectF(0f, 0f, 240f, 320f), RectF(-240f, -320f, 0f, 0f)))
 
         // blockRatio is higher than 0.05 in order to make the smallest display edge (240 dp) 48dp
         // in the pane.
         assertEquals(
-                "{TopologyScale blockRatio=0.200000 originPaneXY=96.0,128.0 paneHeight=256.0}",
+                "{TopologyScale blockRatio=0.200000 originPaneXY=96.0,136.0 paneHeight=272.0}",
                 "" + scale)
 
-        assertPointF(192f, 256f, 0.001f, scale.displayToPaneCoor(480f, 640f))
+        assertPointF(
-        assertPointF(96f, 64f, 0.001f, scale.displayToPaneCoor(0f, -320f))
+            listOf(
-        assertPointF(220f, -430f, 0.001f, scale.paneToDisplayCoor(140f, 42f))
+                PointF(192f, 264f) to scale.displayToPaneCoor(480f, 640f),
+                PointF(96f, 72f) to scale.displayToPaneCoor(0f, -320f),
+                PointF(220f, -470f) to scale.paneToDisplayCoor(140f, 42f),
+            ), 0.001f)
     }
 
     @Test
-    fun paneVerticalPaddingLimitedByTallestDisplay() {
+    fun paneVerticalPaddingSetByMinEdgeLength() {
         val scale = TopologyScale(
                 /* paneWidth= */ 300, minPaneHeight = 0f,
-                minEdgeLength = 48f, maxBlockRatio = 0.05f,
+                minEdgeLength = 48f, maxEdgeLength = 80f,
                 listOf(
                         RectF(0f, 0f, 640f, 480f),
                         RectF(0f, 480f, 640f, 960f),
@@ -108,26 +129,32 @@ class TopologyScaleTest {
                         RectF(0f, 2400f, 640f, 2880f)))
 
         assertEquals(
-                "{TopologyScale blockRatio=0.100000 originPaneXY=118.0,48.0 paneHeight=384.0}",
+                "{TopologyScale blockRatio=0.125000 originPaneXY=110.0,72.0 paneHeight=504.0}",
                 "" + scale)
-        assertPointF(150f, 48f, 0.001f, scale.displayToPaneCoor(320f, 0f))
+        assertPointF(
-        assertPointF(-180f, 2880f, 0.001f, scale.paneToDisplayCoor(100f, 336f))
+            listOf(
+                PointF(150f, 72f) to scale.displayToPaneCoor(320f, 0f),
+                PointF(-80f, 2112f) to scale.paneToDisplayCoor(100f, 336f),
+            ), 0.001f)
     }
 
     @Test
     fun limitedByCustomMaxBlockRatio() {
         val scale = TopologyScale(
                 /* paneWidth= */ 300, minPaneHeight = 0f,
-                minEdgeLength = 24f, maxBlockRatio = 0.12f,
+                minEdgeLength = 24f, maxEdgeLength = 77f,
                 listOf(
                         RectF(0f, 0f, 640f, 480f),
                         RectF(0f, 480f, 640f, 960f)))
 
         assertEquals(
-                "{TopologyScale blockRatio=0.120000 originPaneXY=111.6,57.6 paneHeight=230.4}",
+                "{TopologyScale blockRatio=0.120312 originPaneXY=111.5,36.0 paneHeight=187.5}",
                 "" + scale)
-        assertPointF(150f, 57.6f, 0.001f, scale.displayToPaneCoor(320f, 0f))
+        assertPointF(
-        assertPointF(-96.6667f, 2320f, 0.001f, scale.paneToDisplayCoor(100f, 336f))
+            listOf(
+                PointF(150f, 36.0f) to scale.displayToPaneCoor(320f, 0f),
+                PointF(-95.58442f, 2493.5066f) to scale.paneToDisplayCoor(100f, 336f),
+            ), 0.001f)
     }
 
     @Test
@@ -135,15 +162,18 @@ class TopologyScaleTest {
         // minBlockEdgeLength/minDisplayEdgeLength = 80/480 = 1/6, so the block ratio will be 1/6
         val scale = TopologyScale(
                 /* paneWidth= */ 300, minPaneHeight = 0f,
-                minEdgeLength = 80f, maxBlockRatio = 0.05f,
+                minEdgeLength = 80f, maxEdgeLength = 100f,
                 listOf(
                         RectF(0f, 0f, 640f, 480f),
                         RectF(0f, 480f, 640f, 960f)))
 
         assertEquals(
-                "{TopologyScale blockRatio=0.166667 originPaneXY=96.7,80.0 paneHeight=320.0}",
+                "{TopologyScale blockRatio=0.166667 originPaneXY=96.7,120.0 paneHeight=400.0}",
                 "" + scale)
-        assertPointF(150f, 80f, 0.001f, scale.displayToPaneCoor(320f, 0f))
+        assertPointF(
-        assertPointF(20f, 1536f, 0.001f, scale.paneToDisplayCoor(100f, 336f))
+            listOf(
+                PointF(150f, 120f) to scale.displayToPaneCoor(320f, 0f),
+                PointF(20f, 1296f) to scale.paneToDisplayCoor(100f, 336f),
+            ), 0.001f)
     }
 }