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

Commit c36d21ed authored by Siarhei Vishniakou's avatar Siarhei Vishniakou
Browse files

Ignore resampled values when computing velocity 

VelocityTracker should only operate on the actual data produced by the
touchscreen. It should not use the fake, synthesized data during the
computation.
Using such data can cause unwanted artifacts such as backwards fling,
etc.

Bug: 167946721
Test: m libinput_tests && $ANDROID_HOST_OUT/nativetest64/libinput_tests/libinput_tests
Change-Id: I1ffdc1d38b0b335419f20c7bdd0d4473942bbf57
parent 68950fb9

libs/input/VelocityTracker.cpp

+5 −3
Original line number Diff line number Diff line
@@ -332,11 +332,13 @@ void VelocityTracker::addMovement(const MotionEvent* event) { 
            return;

    }



    size_t historySize = event->getHistorySize();

    const size_t historySize = event->getHistorySize();

    for (size_t h = 0; h <= historySize; h++) {

        nsecs_t eventTime = event->getHistoricalEventTime(h);

        const nsecs_t eventTime = event->getHistoricalEventTime(h);

        for (size_t i = 0; i < event->getPointerCount(); i++) {

            // TODO(b/167946721): skip resampled samples

            if (event->isResampled(i, h)) {

                continue; // skip resampled samples

            }

            const int32_t pointerId = event->getPointerId(i);

            for (int32_t axis : axesToProcess) {

                const float position = event->getHistoricalAxisValue(axis, i, h);

libs/input/tests/VelocityTracker_test.cpp

+45 −4
Original line number Diff line number Diff line
@@ -84,6 +84,8 @@ struct Position { 
    float x;

    float y;



    bool isResampled = false;



    /**

     * If both values are NAN, then this is considered to be an empty entry (no pointer data).

     * If only one of the values is NAN, this is still a valid entry,
@@ -203,10 +205,11 @@ static std::vector<MotionEvent> createTouchMotionEventStream( 


            coords[pointerIndex].clear();

            // We are treating column positions as pointerId

            EXPECT_TRUE(entry.positions[pointerId].isValid()) <<

                    "The entry at pointerId must be valid";

            coords[pointerIndex].setAxisValue(AMOTION_EVENT_AXIS_X, entry.positions[pointerId].x);

            coords[pointerIndex].setAxisValue(AMOTION_EVENT_AXIS_Y, entry.positions[pointerId].y);

            const Position& position = entry.positions[pointerId];

            EXPECT_TRUE(position.isValid()) << "The entry at " << pointerId << " must be valid";

            coords[pointerIndex].setAxisValue(AMOTION_EVENT_AXIS_X, position.x);

            coords[pointerIndex].setAxisValue(AMOTION_EVENT_AXIS_Y, position.y);

            coords[pointerIndex].isResampled = position.isResampled;



            properties[pointerIndex].id = pointerId;

            properties[pointerIndex].toolType = AMOTION_EVENT_TOOL_TYPE_FINGER;
@@ -375,6 +378,44 @@ TEST_F(VelocityTrackerTest, TestComputedVelocity) { 
    EXPECT_FALSE(computedVelocity.getVelocity(AMOTION_EVENT_AXIS_X, MAX_POINTER_ID + 1));

}



/**

 * For a single pointer, the resampled data is ignored.

 */

TEST_F(VelocityTrackerTest, SinglePointerResampledData) {

    std::vector<PlanarMotionEventEntry> motions = {{10ms, {{1, 2}}},

                                                   {20ms, {{2, 4}}},

                                                   {30ms, {{3, 6}}},

                                                   {35ms, {{30, 60, .isResampled = true}}},

                                                   {40ms, {{4, 8}}}};



    computeAndCheckVelocity(VelocityTracker::Strategy::DEFAULT, motions, AMOTION_EVENT_AXIS_X, 100);

    computeAndCheckVelocity(VelocityTracker::Strategy::DEFAULT, motions, AMOTION_EVENT_AXIS_Y, 200);

}



/**

 * For multiple pointers, the resampled data is ignored on a per-pointer basis. If a certain pointer

 * does not have a resampled value, all of the points are used.

 */

TEST_F(VelocityTrackerTest, MultiPointerResampledData) {

    std::vector<PlanarMotionEventEntry> motions = {

            {0ms, {{0, 0}}},

            {10ms, {{1, 0}, {1, 0}}},

            {20ms, {{2, 0}, {2, 0}}},

            {30ms, {{3, 0}, {3, 0}}},

            {35ms, {{30, 0, .isResampled = true}, {30, 0}}},

            {40ms, {{4, 0}, {4, 0}}},

            {45ms, {{5, 0}}}, // ACTION_UP

    };



    // Sample at t=35ms breaks trend. It's marked as resampled for the first pointer, so it should

    // be ignored, and the resulting velocity should be linear. For the second pointer, it's not

    // resampled, so it should cause the velocity to be non-linear.

    computeAndCheckVelocity(VelocityTracker::Strategy::DEFAULT, motions, AMOTION_EVENT_AXIS_X, 100,

                            /*pointerId=*/0);

    computeAndCheckVelocity(VelocityTracker::Strategy::DEFAULT, motions, AMOTION_EVENT_AXIS_X, 3455,

                            /*pointerId=*/1);

}



TEST_F(VelocityTrackerTest, TestGetComputedVelocity) {

    std::vector<PlanarMotionEventEntry> motions = {

            {235089067457000ns, {{528.00, 0}}}, {235089084684000ns, {{527.00, 0}}},