diff options
author | 2023-01-17 09:59:38 -0800 | |
---|---|---|
committer | 2023-01-17 10:11:48 -0800 | |
commit | c36d21edf61e4d3430b3f6b99feca30e13fcfc87 (patch) | |
tree | 2d6c460bd5ff8bfd4728249735ff75873b526544 | |
parent | 68950fb9e6d9a987bb0e849469a491d4242ac1b5 (diff) |
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
-rw-r--r-- | libs/input/VelocityTracker.cpp | 8 | ||||
-rw-r--r-- | libs/input/tests/VelocityTracker_test.cpp | 49 |
2 files changed, 50 insertions, 7 deletions
diff --git a/libs/input/VelocityTracker.cpp b/libs/input/VelocityTracker.cpp index 1cd782d7a7..3632914b93 100644 --- a/libs/input/VelocityTracker.cpp +++ b/libs/input/VelocityTracker.cpp @@ -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); diff --git a/libs/input/tests/VelocityTracker_test.cpp b/libs/input/tests/VelocityTracker_test.cpp index 2678f2f2fd..c6ad3a2218 100644 --- a/libs/input/tests/VelocityTracker_test.cpp +++ b/libs/input/tests/VelocityTracker_test.cpp @@ -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}}}, |