Simplify pane sizing and scaling algorithm

Simplify the scaling parameters and algorithm and reduce vertical
padding.

Originally a maxBlockRatio was specified at .12, which would have
caused a block length to typically be 2560 * 0.12 = 307. However, this
fails to account for smaller displays and high-DPI displays which have
smaller dip dimensions. So instead choose a maximum edge length of
256dp, which I have found to give good results on a laptop device
and a high DPI tablet, and which does not assume typical sizing for
configured displays, and still accomplishes the purpose of limiting
scrolling and dragging size.

Using the aspect ratio of the display bounds in setting the height
frequently caused a very large amount of padding, and limiting based on
the max block height only reduced it slightly. Now we set the vertical
height based on the minEdgeLength. This length is considered the
smallest size that can be comfortably interacted with, and we use it to
give ballpark ideal padding size.

Note that the unit test cases have increased pane height but in
practice (real-world pane width and constraints) this change seems to
decrease the pane height in general.

Flag: com.android.settings.flags.display_topology_pane_in_display_list
Bug: b/352650922
Test: atest DisplayTopologyPreferenceTest.kt
Test: atest TopologyScaleTest.kt
Test: compare appearance on mid-dpi and high-dpi screens with a single 1080p external display, or with two external displays of smallish logical size
Change-Id: Id189892c88a1e833c1f54b0e5447a15f92e3310f

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)
     }
 }