Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit eeec7d0d authored by Matthew DeVore's avatar Matthew DeVore
Browse files

Run body of applyTopology for all non-noop drags 

In onBlockTouchUp, if DisplayTopology.rearrange happened to revert the
change made by the drag so that it matched the before-drag layout, the
blocks would not be moved, so the block would be in the dragged position
but not the normalized position.

This will happen when rearrange has a bug or is otherwise optimizing the
layout, which is dependent on the implementation of rearrange.

The test field mTimesReceivedSameTopology has been replaced with one
that represents an observable positive operation:
mTimesRefreshedBlocks, the validation of which has been added to some
existing tests.

Flag: com.android.settings.flags.display_topology_pane_in_display_list
Test: move display so that rearrange reverts the change, then exit and re-enter the external display fragment, and verify it matches the state when left
Test: DisplayTopologyPreferenceTest
Bug: b/394361999
Bug: b/394355269
Change-Id: Ic3028747d283db77f144831352b7687fe2706391
parent ce463e41

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

+23 −4
Original line number Diff line number Diff line
@@ -334,12 +334,15 @@ class DisplayTopologyPreference(context : Context) 
     * @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

     */

    private data class BlockDrag(

            val stationaryDisps : List<Pair<Int, RectF>>,

            val display: DisplayBlock, val displayId: Int,

            val displayWidth: Float, val displayHeight: Float,

            val dragOffsetX: Float, val dragOffsetY: Float)

            val dragOffsetX: Float, val dragOffsetY: Float,

            var didMove: Boolean = false)



    private var mTopologyInfo : TopologyInfo? = null

    private var mDrag : BlockDrag? = null
@@ -369,7 +372,7 @@ class DisplayTopologyPreference(context : Context) 
        applyTopology(topology)

    }



    @VisibleForTesting var mTimesReceivedSameTopology = 0

    @VisibleForTesting var mTimesRefreshedBlocks = 0



    private fun applyTopology(topology: DisplayTopology) {

        mTopologyHint.text = context.getString(R.string.external_display_topology_hint)
@@ -386,7 +389,6 @@ class DisplayTopologyPreference(context : Context) 
                oldBounds.zip(newBounds).all { (old, new) ->

                    old.first == new.first && sameDisplayPosition(old.second, new.second)

                }) {

            mTimesReceivedSameTopology++

            return

        }
@@ -438,6 +440,7 @@ class DisplayTopologyPreference(context : Context) 
            }

        }

        mPaneContent.removeViews(newBounds.size, recycleableBlocks.size)

        mTimesRefreshedBlocks++



        mTopologyInfo = TopologyInfo(topology, scaling, newBounds)
@@ -481,6 +484,7 @@ class DisplayTopologyPreference(context : Context) 
        val snapRect = clampPosition(drag.stationaryDisps.map { it.second }, dispDragRect)



        drag.display.place(topology.scaling.displayToPaneCoor(snapRect.left, snapRect.top))

        drag.didMove = true



        return true

    }
@@ -491,6 +495,15 @@ class DisplayTopologyPreference(context : Context) 
        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.

            return true

        }



        val newCoor = topology.scaling.paneToDisplayCoor(

                drag.display.x, drag.display.y)

        val newTopology = topology.topology.copy()
@@ -499,9 +512,15 @@ class DisplayTopologyPreference(context : Context) 


        val arr = hashMapOf(*newPositions.toTypedArray())

        newTopology.rearrange(arr)



        // Setting mTopologyInfo to null forces applyTopology to skip the no-op drag check. This is

        // necessary because we don't know if newTopology.rearrange has mutated the topology away

        // from what the user has dragged into position.

        mTopologyInfo = null

        applyTopology(newTopology)



        injector.displayTopology = newTopology



        refreshPane()

        return true

    }

}

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

+37 −3
Original line number Diff line number Diff line
@@ -186,6 +186,8 @@ class DisplayTopologyPreferenceTest { 
    fun dragDisplayDownward() {

        val (leftBlock, _) = setupTwoDisplays()



        preference.mTimesRefreshedBlocks = 0



        val downEvent = MotionEventBuilder.newBuilder()

                .setPointer(0f, 0f)

                .setAction(MotionEvent.ACTION_DOWN)
@@ -208,12 +210,16 @@ class DisplayTopologyPreferenceTest { 
        val child = rootChildren[0]

        assertThat(child.position).isEqualTo(POSITION_LEFT)

        assertThat(child.offset).isWithin(1f).of(82f)



        assertThat(preference.mTimesRefreshedBlocks).isEqualTo(1)

    }



    @Test

    fun dragRootDisplayToNewSide() {

        val (leftBlock, rightBlock) = setupTwoDisplays()



        preference.mTimesRefreshedBlocks = 0



        val downEvent = MotionEventBuilder.newBuilder()

                .setAction(MotionEvent.ACTION_DOWN)

                .setPointer(0f, 0f)
@@ -251,6 +257,32 @@ class DisplayTopologyPreferenceTest { 
        assertThat(paneChildren[0].x)

                .isWithin(1f)

                .of(paneChildren[1].x)



        assertThat(preference.mTimesRefreshedBlocks).isEqualTo(1)

    }



    @Test

    fun noRefreshForUnmovingDrag() {

        val (leftBlock, rightBlock) = setupTwoDisplays()



        preference.mTimesRefreshedBlocks = 0



        val downEvent = MotionEventBuilder.newBuilder()

                .setAction(MotionEvent.ACTION_DOWN)

                .setPointer(0f, 0f)

                .build()



        val upEvent = MotionEventBuilder.newBuilder().setAction(MotionEvent.ACTION_UP).build()



        rightBlock.dispatchTouchEvent(downEvent)

        rightBlock.dispatchTouchEvent(upEvent)



        // After drag, the original views should still be present.

        val paneChildren = getPaneChildren()

        assertThat(paneChildren.indexOf(leftBlock)).isNotEqualTo(-1)

        assertThat(paneChildren.indexOf(rightBlock)).isNotEqualTo(-1)



        assertThat(preference.mTimesRefreshedBlocks).isEqualTo(0)

    }



    @Test
@@ -269,13 +301,15 @@ class DisplayTopologyPreferenceTest { 
    @Test

    fun applyNewTopologyViaListenerUpdate() {

        setupTwoDisplays()



        preference.mTimesRefreshedBlocks = 0

        val newTopology = injector.topology!!.copy()

        newTopology.addDisplay(/* displayId= */ 8008, /* width= */ 300f, /* height= */ 320f)



        injector.topology = newTopology

        injector.topologyListener!!.accept(newTopology)



        assertThat(preference.mTimesReceivedSameTopology).isEqualTo(0)

        assertThat(preference.mTimesRefreshedBlocks).isEqualTo(1)

        val paneChildren = getPaneChildren()

        assertThat(paneChildren).hasSize(3)
@@ -293,11 +327,11 @@ class DisplayTopologyPreferenceTest { 
        injector.topology = twoDisplayTopology(POSITION_TOP, /* offset= */ 12.0f)

        preparePane()



        assertThat(preference.mTimesReceivedSameTopology).isEqualTo(0)

        preference.mTimesRefreshedBlocks = 0

        injector.topology = twoDisplayTopology(POSITION_TOP, /* offset= */ 12.1f)

        injector.topologyListener!!.accept(injector.topology!!)



        assertThat(preference.mTimesReceivedSameTopology).isEqualTo(1)

        assertThat(preference.mTimesRefreshedBlocks).isEqualTo(0)

    }



    @Test