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

Commit 44d065f8 authored by Yongshun Liu's avatar Yongshun Liu
Browse files

a11y: Fix accumulated drag offset for magnification UI 

Draggable magnification UI components, such as the magnification window,
the mode switch button, and the settings panel, would gradually drift
from the pointer's actual position during drag gestures.

This was caused by floating-point precision loss. The drag offsets were
calculated as floats, but the UI position updates used integers. The
repeated truncation of the fractional part of the offset during casting
led to an accumulating error.

This change fixes the issue by preserving the fractional part of the
drag offset between motion events. The `MotionAccumulator` now only
consumes the integer part of the offset for the current UI update,
carrying over the remaining fractional part to the next event. This
ensures that the full drag distance is accounted for over time,
eliminating the deviation.

Additionally, this change refactors the `OnGestureListener` interface
by removing the unused coordinate parameters from `onStart()` and
`onFinish()`, simplifying the API.

Bug: 436696444
Flag: EXEMPT bugfix
Test: atest SystemUITests:MagnificationGestureDetectorTest
Test: atest SystemUITests:WindowMagnificationControllerTest
Change-Id: I0a5dd67b44a6a8acb610acb532478d0d1eafbeb6
parent 72f87516

packages/SystemUI/multivalentTests/src/com/android/systemui/accessibility/MagnificationGestureDetectorTest.java

+63 −14
Original line number Diff line number Diff line
@@ -16,12 +16,14 @@ 


package com.android.systemui.accessibility;



import static org.junit.Assert.assertEquals;

import static org.mockito.ArgumentMatchers.any;

import static org.mockito.ArgumentMatchers.anyFloat;

import static org.mockito.ArgumentMatchers.anyInt;

import static org.mockito.ArgumentMatchers.anyLong;

import static org.mockito.ArgumentMatchers.eq;

import static org.mockito.Mockito.doAnswer;

import static org.mockito.Mockito.never;

import static org.mockito.Mockito.times;

import static org.mockito.Mockito.verify;



import android.os.Handler;
@@ -43,6 +45,7 @@ import org.junit.After; 
import org.junit.Before;

import org.junit.Test;

import org.junit.runner.RunWith;

import org.mockito.ArgumentCaptor;

import org.mockito.InOrder;

import org.mockito.Mock;

import org.mockito.Mockito;
@@ -52,8 +55,8 @@ import org.mockito.MockitoAnnotations; 
@RunWith(AndroidJUnit4.class)

public class MagnificationGestureDetectorTest extends SysuiTestCase {



    private static final float ACTION_DOWN_X = 100;

    private static final float ACTION_DOWN_Y = 200;

    private static final int ACTION_DOWN_X = 100;

    private static final int ACTION_DOWN_Y = 200;

    private final int mTouchSlop = ViewConfiguration.get(getContext()).getScaledTouchSlop();

    private MagnificationGestureDetector mGestureDetector;

    private final MotionEventHelper mMotionEventHelper = new MotionEventHelper();
@@ -88,7 +91,7 @@ public class MagnificationGestureDetectorTest extends SysuiTestCase { 


        mGestureDetector.onTouch(mSpyView, downEvent);



        mListener.onStart(ACTION_DOWN_X, ACTION_DOWN_Y);

        verify(mListener).onStart();

    }



    @Test
@@ -103,10 +106,10 @@ public class MagnificationGestureDetectorTest extends SysuiTestCase { 
        mGestureDetector.onTouch(mSpyView, upEvent);



        InOrder inOrder = Mockito.inOrder(mListener);

        inOrder.verify(mListener).onStart(ACTION_DOWN_X, ACTION_DOWN_Y);

        inOrder.verify(mListener).onStart();

        inOrder.verify(mListener).onSingleTap(mSpyView);

        inOrder.verify(mListener).onFinish(ACTION_DOWN_X, ACTION_DOWN_Y);

        verify(mListener, never()).onDrag(eq(mSpyView), anyFloat(), anyFloat());

        inOrder.verify(mListener).onFinish();

        verify(mListener, never()).onDrag(eq(mSpyView), anyInt(), anyInt());

    }



    @Test
@@ -182,15 +185,15 @@ public class MagnificationGestureDetectorTest extends SysuiTestCase { 
        mGestureDetector.onTouch(mSpyView, upEvent);



        InOrder inOrder = Mockito.inOrder(mListener);

        inOrder.verify(mListener).onStart(ACTION_DOWN_X, ACTION_DOWN_Y);

        inOrder.verify(mListener).onFinish(ACTION_DOWN_X, ACTION_DOWN_Y);

        inOrder.verify(mListener).onStart();

        inOrder.verify(mListener).onFinish();

        verify(mListener, never()).onSingleTap(mSpyView);

    }



    @Test

    public void performDrag_invokeCallbacksInOrder() {

        final long downTime = SystemClock.uptimeMillis();

        final float dragOffset = mTouchSlop + 10;

        final int dragOffset = mTouchSlop + 10;

        final MotionEvent downEvent = mMotionEventHelper.obtainMotionEvent(downTime, downTime,

                MotionEvent.ACTION_DOWN, ACTION_DOWN_X, ACTION_DOWN_Y);

        final MotionEvent moveEvent = mMotionEventHelper.obtainMotionEvent(downTime, downTime,
@@ -203,9 +206,9 @@ public class MagnificationGestureDetectorTest extends SysuiTestCase { 
        mGestureDetector.onTouch(mSpyView, upEvent);



        InOrder inOrder = Mockito.inOrder(mListener);

        inOrder.verify(mListener).onStart(ACTION_DOWN_X, ACTION_DOWN_Y);

        inOrder.verify(mListener).onStart();

        inOrder.verify(mListener).onDrag(mSpyView, dragOffset, 0);

        inOrder.verify(mListener).onFinish(ACTION_DOWN_X, ACTION_DOWN_Y);

        inOrder.verify(mListener).onFinish();

        verify(mListener, never()).onSingleTap(mSpyView);

    }
@@ -239,8 +242,54 @@ public class MagnificationGestureDetectorTest extends SysuiTestCase { 
        mGestureDetector.onTouch(mSpyView, moveEvent);

        mGestureDetector.onTouch(mSpyView, upEvent);



        verify(mListener).onStart(ACTION_DOWN_X, ACTION_DOWN_Y);

        verify(mListener).onStart();

        verify(mListener).onDrag(mSpyView, mTouchSlop + 30, 0);

        verify(mListener).onFinish(ACTION_DOWN_X, ACTION_DOWN_Y);

        verify(mListener).onFinish();

    }



    @Test

    public void dragWithFractionalOffsets_totalDragOffsetIsCorrect() {

        final long downTime = SystemClock.uptimeMillis();

        final float dragOffsetX = 0.6f;

        final float dragOffsetY = -0.6f;

        final int dragCount = 4;

        final float startX = ACTION_DOWN_X;

        final float startY = ACTION_DOWN_Y;



        final MotionEvent downEvent = mMotionEventHelper.obtainMotionEvent(downTime, downTime,

                MotionEvent.ACTION_DOWN, startX, startY);

        mGestureDetector.onTouch(mSpyView, downEvent);



        final float firstMoveX = startX + mTouchSlop + 1;

        final float firstMoveY = startY - mTouchSlop - 1;

        final MotionEvent firstMoveEvent = mMotionEventHelper.obtainMotionEvent(downTime, downTime,

                MotionEvent.ACTION_MOVE, firstMoveX, firstMoveY);

        mGestureDetector.onTouch(mSpyView, firstMoveEvent);



        for (int i = 1; i <= dragCount; i++) {

            final MotionEvent moveEvent = mMotionEventHelper.obtainMotionEvent(downTime, downTime,

                    MotionEvent.ACTION_MOVE, firstMoveX + dragOffsetX * i,

                    firstMoveY + dragOffsetY * i);

            mGestureDetector.onTouch(mSpyView, moveEvent);

        }



        final MotionEvent upEvent = mMotionEventHelper.obtainMotionEvent(downTime, downTime,

                MotionEvent.ACTION_UP, firstMoveX + dragOffsetX * dragCount,

                firstMoveY + dragOffsetY * dragCount);

        mGestureDetector.onTouch(mSpyView, upEvent);



        ArgumentCaptor<Integer> offsetXCaptor = ArgumentCaptor.forClass(Integer.class);

        ArgumentCaptor<Integer> offsetYCaptor = ArgumentCaptor.forClass(Integer.class);

        verify(mListener, times(dragCount + 1)).onDrag(eq(mSpyView), offsetXCaptor.capture(),

                offsetYCaptor.capture());



        int totalOffsetX = offsetXCaptor.getAllValues().stream().mapToInt(Integer::intValue).sum();

        int totalOffsetY = offsetYCaptor.getAllValues().stream().mapToInt(Integer::intValue).sum();

        int expectedTotalOffsetX = (int) (mTouchSlop + 1 + dragOffsetX * dragCount);

        int expectedTotalOffsetY = (int) (-mTouchSlop - 1 + dragOffsetY * dragCount);

        assertEquals("Total X offset should be accumulated correctly",

                expectedTotalOffsetX, totalOffsetX);

        assertEquals("Total Y offset should be accumulated correctly",

                expectedTotalOffsetY, totalOffsetY);

    }

}

packages/SystemUI/src/com/android/systemui/accessibility/MagnificationGestureDetector.java

+95 −41
Original line number Diff line number Diff line
@@ -19,6 +19,7 @@ package com.android.systemui.accessibility; 
import android.annotation.DisplayContext;

import android.annotation.NonNull;

import android.content.Context;

import android.graphics.Point;

import android.graphics.PointF;

import android.os.Handler;

import android.view.Display;
@@ -56,36 +57,32 @@ class MagnificationGestureDetector { 
         * @param offsetY The Y offset in screen coordinate.

         * @return {@code true} if this gesture is handled.

         */

        boolean onDrag(View view, float offsetX, float offsetY);

        boolean onDrag(View view, int offsetX, int offsetY);



        /**

         * Notified when a tap occurs with the down {@link MotionEvent} that triggered it. This will

         * be triggered immediately for every down event. All other events should be preceded by

         * this.

         *

         * @param x The X coordinate of the down event.

         * @param y The Y coordinate of the down event.

         * @return {@code true} if the down event is handled, otherwise the events won't be sent to

         * the view.

         */

        boolean onStart(float x, float y);

        boolean onStart();



        /**

         * Called when the detection is finished. In other words, it is called when up/cancel {@link

         * MotionEvent} is received. It will be triggered after single-tap

         * MotionEvent} is received. It will be triggered after single-tap.

         *

         * @param x The X coordinate on the screen of the up event or the cancel event.

         * @param y The Y coordinate on the screen of the up event or the cancel event.

         * @return {@code true} if the event is handled.

         */

        boolean onFinish(float x, float y);

        boolean onFinish();

    }



    private final MotionAccumulator mAccumulator = new MotionAccumulator();

    @NonNull

    private final MotionAccumulator mAccumulator;

    private final Handler mHandler;

    private final Runnable mCancelTapGestureRunnable;

    private final OnGestureListener mOnGestureListener;

    private final int mTouchSlopSquare;

    // Assume the gesture default is a single-tap. Set it to false if the gesture couldn't be a

    // single-tap anymore.

    private boolean mDetectSingleTap = true;
@@ -98,8 +95,7 @@ class MagnificationGestureDetector { 
     */

    MagnificationGestureDetector(@DisplayContext Context context, @NonNull Handler handler,

            @NonNull OnGestureListener listener) {

        final int touchSlop = ViewConfiguration.get(context).getScaledTouchSlop();

        mTouchSlopSquare = touchSlop * touchSlop;

        mAccumulator = new MotionAccumulator(ViewConfiguration.get(context).getScaledTouchSlop());

        mHandler = handler;

        mOnGestureListener = listener;

        mCancelTapGestureRunnable = () -> mDetectSingleTap = false;
@@ -113,15 +109,13 @@ class MagnificationGestureDetector { 
     * @return {@code True} if the {@link OnGestureListener} consumes the event, else false.

     */

    boolean onTouch(View view, MotionEvent event) {

        final float rawX = event.getRawX();

        final float rawY = event.getRawY();

        mAccumulator.onMotionEvent(event);

        boolean handled = false;

        switch (event.getActionMasked()) {

            case MotionEvent.ACTION_DOWN:

                mHandler.postAtTime(mCancelTapGestureRunnable,

                        event.getDownTime() + ViewConfiguration.getLongPressTimeout());

                handled |= mOnGestureListener.onStart(rawX, rawY);

                handled |= mOnGestureListener.onStart();

                break;

            case MotionEvent.ACTION_POINTER_DOWN:

                stopSingleTapDetection();
@@ -137,7 +131,7 @@ class MagnificationGestureDetector { 
                }

                // Fall through

            case MotionEvent.ACTION_CANCEL:

                handled |= mOnGestureListener.onFinish(rawX, rawY);

                handled |= mOnGestureListener.onFinish();

                reset();

                break;

        }
@@ -149,14 +143,7 @@ class MagnificationGestureDetector { 
            return;

        }



        final float deltaX = mAccumulator.getDeltaX();

        final float deltaY = mAccumulator.getDeltaY();

        if (Float.isNaN(deltaX) || Float.isNaN(deltaY)) {

            return;

        }



        final float distanceSquare = (deltaX * deltaX) + (deltaY * deltaY);

        if (distanceSquare > mTouchSlopSquare) {

        if (mAccumulator.isDraggingDetected()) {

            mDraggingDetected = true;

            stopSingleTapDetection();

        }
@@ -171,10 +158,8 @@ class MagnificationGestureDetector { 
        if (!mDraggingDetected) {

            return false;

        }

        final float deltaX = mAccumulator.getDeltaX();

        final float deltaY = mAccumulator.getDeltaY();

        mAccumulator.consumeDelta();

        return mOnGestureListener.onDrag(view, deltaX, deltaY);

        final Point delta = mAccumulator.getAndConsumeDelta();

        return mOnGestureListener.onDrag(view, delta.x, delta.y);

    }



    private void reset() {
@@ -184,12 +169,40 @@ class MagnificationGestureDetector { 
        mDraggingDetected = false;

    }



    /**

     * A helper class to accumulate raw motion events and determine if a dragging gesture is

     * happening. It provides the delta between events for the client to perform dragging actions.

     *

     * <p>For dragging actions, the UI uses integer values for pixel offsets. This class accumulates

     * the gesture's relative offset as a floating-point value. To avoid accumulated errors from

     * float-to-int conversions, the class keeps the fractional part of the offset internally. It

     * only reports the integer part of the offset to event handlers via {@link #getDeltaX()} and

     * {@link #getDeltaY()}, and the consumed integer offset is then subtracted from the internal

     * accumulated offset (see b/436696444).

     */

    private static class MotionAccumulator {

        private final PointF mAccumulatedDelta = new PointF(Float.NaN, Float.NaN);

        private final PointF mLastLocation = new PointF(Float.NaN, Float.NaN);

        private final int mTouchSlopSquare;



        /**

         * @param touchSlop Distance a touch can wander before becoming a drag.

         */

        MotionAccumulator(int touchSlop) {

            mTouchSlopSquare = touchSlop * touchSlop;

        }



        // Start or accumulate the motion event location.

        public void onMotionEvent(MotionEvent event) {

        /**

         * Processes a {@link MotionEvent} to accumulate the gesture's deltas.

         *

         * <p>This method tracks the movement difference between events. For touch events, it's the

         * change in raw screen coordinates. For mouse events, it uses the relative motion axes

         * ({@link MotionEvent#AXIS_RELATIVE_X} and {@link MotionEvent#AXIS_RELATIVE_Y}) to support

         * dragging even when the pointer is at the screen edge.

         *

         * @param event The motion event to process.

         */

        void onMotionEvent(MotionEvent event) {

            switch (event.getActionMasked()) {

                case MotionEvent.ACTION_DOWN:

                    mAccumulatedDelta.set(0, 0);
@@ -219,23 +232,64 @@ class MagnificationGestureDetector { 
            }

        }



        // Get delta X of accumulated motions, or NaN if no motion is added.

        public float getDeltaX() {

            return mAccumulatedDelta.x;

        /**

         * Gets the delta X of accumulated motions, or zero if no motion is added.

         *

         * <p>Please note, when reporting delta offset, it uses casting so that the offset is always

         * truncated and the fractional part could be accumulated and used with future moves.

         *

         * @return The X offset of the accumulated motions.

         */

        int getDeltaX() {

            return (int) mAccumulatedDelta.x;

        }



        // Get delta Y of accumulated motions, or NaN if no motion is added.

        public float getDeltaY() {

            return mAccumulatedDelta.y;

        /**

         * Gets the delta Y of accumulated motions, or zero if no motion is added.

         *

         * <p>Please note, when reporting delta offset, it uses casting so that the offset is always

         * truncated and the fractional part could be accumulated and used with future moves.

         *

         * @return The Y offset of the accumulated motions.

         */

        int getDeltaY() {

            return (int) mAccumulatedDelta.y;

        }



        // Consume the accumulated motions, and restart accumulation from the last added motion.

        public void consumeDelta() {

            mAccumulatedDelta.set(0, 0);

        /**

         * Returns whether a dragging gesture has been detected.

         *

         * @return {@code true} if a drag has been detected, {@code false} otherwise.

         */

        boolean isDraggingDetected() {

            if (Float.isNaN(mAccumulatedDelta.x) || Float.isNaN(mAccumulatedDelta.y)) {

                return false;

            }



            final float distanceSquare = (mAccumulatedDelta.x * mAccumulatedDelta.x)

                    + (mAccumulatedDelta.y * mAccumulatedDelta.y);

            if (distanceSquare > mTouchSlopSquare) {

                return true;

            }



            return false;

        }



        /**

         * Gets the integer part of the accumulated motion delta and consumes it, leaving the

         * fractional part for the next calculation.

         *

         * @return A {@link Point} containing the (x, y) integer delta.

         */

        @NonNull

        Point getAndConsumeDelta() {

            final Point delta = new Point(getDeltaX(), getDeltaY());

            mAccumulatedDelta.offset(-delta.x, -delta.y);

            return delta;

        }



        // Reset the state.

        public void reset() {

        /** Resets the accumulator to its initial state. */

        void reset() {

            resetPointF(mAccumulatedDelta);

            resetPointF(mLastLocation);

        }

packages/SystemUI/src/com/android/systemui/accessibility/MagnificationModeSwitch.java

+4 −4
Original line number Diff line number Diff line
@@ -229,19 +229,19 @@ class MagnificationModeSwitch implements MagnificationGestureDetector.OnGestureL 
    }



    @Override

    public boolean onDrag(View v, float offsetX, float offsetY) {

    public boolean onDrag(View v, int offsetX, int offsetY) {

        moveButton(offsetX, offsetY);

        return true;

    }



    @Override

    public boolean onStart(float x, float y) {

    public boolean onStart() {

        stopFadeOutAnimation();

        return true;

    }



    @Override

    public boolean onFinish(float xOffset, float yOffset) {

    public boolean onFinish() {

        if (mIsVisible) {

            final int windowWidth = mWindowManager.getCurrentWindowMetrics().getBounds().width();

            final int halfWindowWidth = windowWidth / 2;
@@ -261,7 +261,7 @@ class MagnificationModeSwitch implements MagnificationGestureDetector.OnGestureL 
        updateButtonViewLayoutIfNeeded();

    }



    private void moveButton(float offsetX, float offsetY) {

    private void moveButton(int offsetX, int offsetY) {

        mSfVsyncFrameProvider.postFrameCallback(l -> {

            mParams.x += offsetX;

            mParams.y += offsetY;

packages/SystemUI/src/com/android/systemui/accessibility/WindowMagnificationController.java

+12 −13
Original line number Diff line number Diff line
@@ -1470,11 +1470,11 @@ class WindowMagnificationController implements View.OnTouchListener, SurfaceHold 
    }



    @Override

    public boolean onDrag(View view, float offsetX, float offsetY) {

    public boolean onDrag(View view, int offsetX, int offsetY) {

        if (mEditSizeEnable) {

            return changeWindowSize(view, offsetX, offsetY);

        } else {

            move((int) offsetX, (int) offsetY);

            move(offsetX, offsetY);

        }

        return true;

    }
@@ -1521,7 +1521,7 @@ class WindowMagnificationController implements View.OnTouchListener, SurfaceHold 
        }

    }



    private boolean changeWindowSize(View view, float offsetX, float offsetY) {

    private boolean changeWindowSize(View view, int offsetX, int offsetY) {

        if (view == mLeftDrag) {

            changeMagnificationFrameSize(offsetX, 0, 0, 0);

        } else if (view == mRightDrag) {
@@ -1546,8 +1546,7 @@ class WindowMagnificationController implements View.OnTouchListener, SurfaceHold 
    }



    private void changeMagnificationFrameSize(

            float leftOffset, float topOffset, float rightOffset,

            float bottomOffset) {

            int leftOffset, int topOffset, int rightOffset, int bottomOffset) {

        boolean bRTL = isRTL(mContext);

        final int initSize = Math.min(mWindowBounds.width(), mWindowBounds.height()) / 3;
@@ -1573,14 +1572,14 @@ class WindowMagnificationController implements View.OnTouchListener, SurfaceHold 
        tempRect.set(mMagnificationFrame);



        if (bRTL) {

            tempRect.left += (int) (rightOffset);

            tempRect.right += (int) (leftOffset);

            tempRect.left += rightOffset;

            tempRect.right += leftOffset;

        } else {

            tempRect.right += (int) (rightOffset);

            tempRect.left += (int) (leftOffset);

            tempRect.right += rightOffset;

            tempRect.left += leftOffset;

        }

        tempRect.top += (int) (topOffset);

        tempRect.bottom += (int) (bottomOffset);

        tempRect.top += topOffset;

        tempRect.bottom += bottomOffset;



        if (tempRect.width() < initSize || tempRect.height() < initSize

                || tempRect.width() > maxWidthSize || tempRect.height() > maxHeightSize) {
@@ -1604,13 +1603,13 @@ class WindowMagnificationController implements View.OnTouchListener, SurfaceHold 
    }



    @Override

    public boolean onStart(float x, float y) {

    public boolean onStart() {

        mIsDragging = true;

        return true;

    }



    @Override

    public boolean onFinish(float x, float y) {

    public boolean onFinish() {

        maybeRepositionButton();

        mIsDragging = false;

        return false;

packages/SystemUI/src/com/android/systemui/accessibility/WindowMagnificationSettings.java

+4 −4
Original line number Diff line number Diff line
@@ -307,18 +307,18 @@ class WindowMagnificationSettings implements MagnificationGestureDetector.OnGest 
    }



    @Override

    public boolean onDrag(View v, float offsetX, float offsetY) {

    public boolean onDrag(View v, int offsetX, int offsetY) {

        moveButton(offsetX, offsetY);

        return true;

    }



    @Override

    public boolean onStart(float x, float y) {

    public boolean onStart() {

        return true;

    }



    @Override

    public boolean onFinish(float xOffset, float yOffset) {

    public boolean onFinish() {

        if (!mSingleTapDetected) {

            showSettingPanel();

        }
@@ -331,7 +331,7 @@ class WindowMagnificationSettings implements MagnificationGestureDetector.OnGest 
        return mSettingView;

    }



    private void moveButton(float offsetX, float offsetY) {

    private void moveButton(int offsetX, int offsetY) {

        mSfVsyncFrameProvider.postFrameCallback(l -> {

            mParams.x += offsetX;

            mParams.y += offsetY;