summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
author Paul Ramirez <peramirez@google.com> 2024-09-30 01:39:00 +0000
committer Paul Ramirez <peramirez@google.com> 2024-10-09 00:58:05 +0000
commit4d3b03adfa3543158c982546f3d6daec7eed06e8 (patch)
treea9878f6dd922e27378fa98ea894ab399457f4c0b
parent7f1efed1f8fcae76c021bfcea244c4f92844a608 (diff)
Add logic to overwrite pointer coordinates in motion event
Included ResampledValueIsUsedForIdenticalCoordinates from TouchResampling_test.cpp, and added the missing logic in LegacyResampler to pass the test. The CL wrongly assumes pointer information guarantees between motion events, that is, pointer IDs can appear in different order between samples. This issue is fixed in the second to last CL in the relation chain by using an associative array as a data structure to store and access pointer properties and coordinates. Bug: 297226446 Flag: EXEMPT refactor Test: TEST=libinput_tests; m $TEST && $ANDROID_HOST_OUT/nativetest64/$TEST/$TEST --gtest_filter="InputConsumerResamplingTest*" Change-Id: I12eb8eae3b3389cfb5449cc24a785bd9d9a6d280
-rw-r--r--include/input/Resampler.h22
-rw-r--r--libs/input/Resampler.cpp72
-rw-r--r--libs/input/tests/InputConsumerResampling_test.cpp67
3 files changed, 140 insertions, 21 deletions
diff --git a/include/input/Resampler.h b/include/input/Resampler.h
index da0c5b2150..f04dfde995 100644
--- a/include/input/Resampler.h
+++ b/include/input/Resampler.h
@@ -100,6 +100,17 @@ private:
RingBuffer<Sample> mLatestSamples{/*capacity=*/2};
/**
+ * Latest sample in mLatestSamples after resampling motion event. Used to compare if a pointer
+ * does not move between samples.
+ */
+ std::optional<Sample> mLastRealSample;
+
+ /**
+ * Latest prediction. Used to overwrite motion event samples if a set of conditions is met.
+ */
+ std::optional<Sample> mPreviousPrediction;
+
+ /**
* Adds up to mLatestSamples.capacity() of motionEvent's latest samples to mLatestSamples. If
* motionEvent has fewer samples than mLatestSamples.capacity(), then the available samples are
* added to mLatestSamples.
@@ -144,6 +155,17 @@ private:
*/
std::optional<Sample> attemptExtrapolation(std::chrono::nanoseconds resampleTime) const;
+ /**
+ * Iterates through motion event samples, and calls overwriteStillPointers on each sample.
+ */
+ void overwriteMotionEventSamples(MotionEvent& motionEvent) const;
+
+ /**
+ * Overwrites with resampled data the pointer coordinates that did not move between motion event
+ * samples, that is, both x and y values are identical to mLastRealSample.
+ */
+ void overwriteStillPointers(MotionEvent& motionEvent, size_t sampleIndex) const;
+
inline static void addSampleToMotionEvent(const Sample& sample, MotionEvent& motionEvent);
};
} // namespace android
diff --git a/libs/input/Resampler.cpp b/libs/input/Resampler.cpp
index 1adff7ba36..8fe904f9a4 100644
--- a/libs/input/Resampler.cpp
+++ b/libs/input/Resampler.cpp
@@ -18,6 +18,7 @@
#include <algorithm>
#include <chrono>
+#include <ostream>
#include <android-base/logging.h>
#include <android-base/properties.h>
@@ -26,10 +27,7 @@
#include <input/Resampler.h>
#include <utils/Timers.h>
-using std::chrono::nanoseconds;
-
namespace android {
-
namespace {
const bool IS_DEBUGGABLE_BUILD =
@@ -49,6 +47,8 @@ bool debugResampling() {
return __android_log_is_loggable(ANDROID_LOG_DEBUG, LOG_TAG "Resampling", ANDROID_LOG_INFO);
}
+using std::chrono::nanoseconds;
+
constexpr std::chrono::milliseconds RESAMPLE_LATENCY{5};
constexpr std::chrono::milliseconds RESAMPLE_MIN_DELTA{2};
@@ -75,6 +75,31 @@ PointerCoords calculateResampledCoords(const PointerCoords& a, const PointerCoor
resampledCoords.setAxisValue(AMOTION_EVENT_AXIS_Y, lerp(a.getY(), b.getY(), alpha));
return resampledCoords;
}
+
+bool equalXY(const PointerCoords& a, const PointerCoords& b) {
+ return (a.getX() == b.getX()) && (a.getY() == b.getY());
+}
+
+void setMotionEventPointerCoords(MotionEvent& motionEvent, size_t sampleIndex, size_t pointerIndex,
+ const PointerCoords& pointerCoords) {
+ // Ideally, we should not cast away const. In this particular case, it's safe to cast away const
+ // and dereference getHistoricalRawPointerCoords returned pointer because motionEvent is a
+ // nonconst reference to a MotionEvent object, so mutating the object should not be undefined
+ // behavior; moreover, the invoked method guarantees to return a valid pointer. Otherwise, it
+ // fatally logs. Alternatively, we could've created a new MotionEvent from scratch, but this
+ // approach is simpler and more efficient.
+ PointerCoords& motionEventCoords = const_cast<PointerCoords&>(
+ *(motionEvent.getHistoricalRawPointerCoords(pointerIndex, sampleIndex)));
+ motionEventCoords.setAxisValue(AMOTION_EVENT_AXIS_X, pointerCoords.getX());
+ motionEventCoords.setAxisValue(AMOTION_EVENT_AXIS_Y, pointerCoords.getY());
+ motionEventCoords.isResampled = pointerCoords.isResampled;
+}
+
+std::ostream& operator<<(std::ostream& os, const PointerCoords& pointerCoords) {
+ os << "(" << pointerCoords.getX() << ", " << pointerCoords.getY() << ")";
+ return os;
+}
+
} // namespace
void LegacyResampler::updateLatestSamples(const MotionEvent& motionEvent) {
@@ -85,12 +110,9 @@ void LegacyResampler::updateLatestSamples(const MotionEvent& motionEvent) {
std::vector<Pointer> pointers;
const size_t numPointers = motionEvent.getPointerCount();
for (size_t pointerIndex = 0; pointerIndex < numPointers; ++pointerIndex) {
- // getSamplePointerCoords is the vector representation of a getHistorySize by
- // getPointerCount matrix.
- const PointerCoords& pointerCoords =
- motionEvent.getSamplePointerCoords()[sampleIndex * numPointers + pointerIndex];
- pointers.push_back(
- Pointer{*motionEvent.getPointerProperties(pointerIndex), pointerCoords});
+ pointers.push_back(Pointer{*(motionEvent.getPointerProperties(pointerIndex)),
+ *(motionEvent.getHistoricalRawPointerCoords(pointerIndex,
+ sampleIndex))});
}
mLatestSamples.pushBack(
Sample{nanoseconds{motionEvent.getHistoricalEventTime(sampleIndex)}, pointers});
@@ -245,6 +267,28 @@ nanoseconds LegacyResampler::getResampleLatency() const {
return RESAMPLE_LATENCY;
}
+void LegacyResampler::overwriteMotionEventSamples(MotionEvent& motionEvent) const {
+ const size_t numSamples = motionEvent.getHistorySize() + 1;
+ for (size_t sampleIndex = 0; sampleIndex < numSamples; ++sampleIndex) {
+ overwriteStillPointers(motionEvent, sampleIndex);
+ }
+}
+
+void LegacyResampler::overwriteStillPointers(MotionEvent& motionEvent, size_t sampleIndex) const {
+ for (size_t pointerIndex = 0; pointerIndex < motionEvent.getPointerCount(); ++pointerIndex) {
+ const PointerCoords& pointerCoords =
+ *(motionEvent.getHistoricalRawPointerCoords(pointerIndex, sampleIndex));
+ if (equalXY(mLastRealSample->pointers[pointerIndex].coords, pointerCoords)) {
+ LOG_IF(INFO, debugResampling())
+ << "Pointer ID: " << motionEvent.getPointerId(pointerIndex)
+ << " did not move. Overwriting its coordinates from " << pointerCoords << " to "
+ << mLastRealSample->pointers[pointerIndex].coords;
+ setMotionEventPointerCoords(motionEvent, sampleIndex, pointerIndex,
+ mPreviousPrediction->pointers[pointerIndex].coords);
+ }
+ }
+}
+
void LegacyResampler::resampleMotionEvent(nanoseconds frameTime, MotionEvent& motionEvent,
const InputMessage* futureSample) {
const nanoseconds resampleTime = frameTime - RESAMPLE_LATENCY;
@@ -261,6 +305,16 @@ void LegacyResampler::resampleMotionEvent(nanoseconds frameTime, MotionEvent& mo
: (attemptExtrapolation(resampleTime));
if (sample.has_value()) {
addSampleToMotionEvent(*sample, motionEvent);
+ if (mPreviousPrediction.has_value()) {
+ overwriteMotionEventSamples(motionEvent);
+ }
+ // mPreviousPrediction is only updated whenever extrapolation occurs because extrapolation
+ // is about predicting upcoming scenarios.
+ if (futureSample == nullptr) {
+ mPreviousPrediction = sample;
+ }
}
+ mLastRealSample = *(mLatestSamples.end() - 1);
}
+
} // namespace android
diff --git a/libs/input/tests/InputConsumerResampling_test.cpp b/libs/input/tests/InputConsumerResampling_test.cpp
index b139e8766f..85311afea9 100644
--- a/libs/input/tests/InputConsumerResampling_test.cpp
+++ b/libs/input/tests/InputConsumerResampling_test.cpp
@@ -197,8 +197,6 @@ TEST_F(InputConsumerResamplingTest, EventIsResampled) {
mClientTestChannel->enqueueMessage(nextPointerMessage(
{0ms, {Pointer{.id = 0, .x = 10.0f, .y = 20.0f}}, AMOTION_EVENT_ACTION_DOWN}));
- mClientTestChannel->assertNoSentMessages();
-
invokeLooperCallback();
assertReceivedMotionEvent({InputEventEntry{0ms,
{Pointer{.id = 0, .x = 10.0f, .y = 20.0f}},
@@ -238,8 +236,6 @@ TEST_F(InputConsumerResamplingTest, EventIsResampledWithDifferentId) {
mClientTestChannel->enqueueMessage(nextPointerMessage(
{0ms, {Pointer{.id = 1, .x = 10.0f, .y = 20.0f}}, AMOTION_EVENT_ACTION_DOWN}));
- mClientTestChannel->assertNoSentMessages();
-
invokeLooperCallback();
assertReceivedMotionEvent({InputEventEntry{0ms,
{Pointer{.id = 1, .x = 10.0f, .y = 20.0f}},
@@ -280,8 +276,6 @@ TEST_F(InputConsumerResamplingTest, StylusEventIsResampled) {
{Pointer{.id = 0, .x = 10.0f, .y = 20.0f, .toolType = ToolType::STYLUS}},
AMOTION_EVENT_ACTION_DOWN}));
- mClientTestChannel->assertNoSentMessages();
-
invokeLooperCallback();
assertReceivedMotionEvent({InputEventEntry{0ms,
{Pointer{.id = 0,
@@ -338,8 +332,6 @@ TEST_F(InputConsumerResamplingTest, MouseEventIsResampled) {
{Pointer{.id = 0, .x = 10.0f, .y = 20.0f, .toolType = ToolType::MOUSE}},
AMOTION_EVENT_ACTION_DOWN}));
- mClientTestChannel->assertNoSentMessages();
-
invokeLooperCallback();
assertReceivedMotionEvent({InputEventEntry{0ms,
{Pointer{.id = 0,
@@ -396,8 +388,6 @@ TEST_F(InputConsumerResamplingTest, PalmEventIsNotResampled) {
{Pointer{.id = 0, .x = 10.0f, .y = 20.0f, .toolType = ToolType::PALM}},
AMOTION_EVENT_ACTION_DOWN}));
- mClientTestChannel->assertNoSentMessages();
-
invokeLooperCallback();
assertReceivedMotionEvent(
{InputEventEntry{0ms,
@@ -438,8 +428,6 @@ TEST_F(InputConsumerResamplingTest, SampleTimeEqualsEventTime) {
mClientTestChannel->enqueueMessage(nextPointerMessage(
{0ms, {Pointer{.id = 0, .x = 10.0f, .y = 20.0f}}, AMOTION_EVENT_ACTION_DOWN}));
- mClientTestChannel->assertNoSentMessages();
-
invokeLooperCallback();
assertReceivedMotionEvent({InputEventEntry{0ms,
{Pointer{.id = 0, .x = 10.0f, .y = 20.0f}},
@@ -468,4 +456,59 @@ TEST_F(InputConsumerResamplingTest, SampleTimeEqualsEventTime) {
mClientTestChannel->assertFinishMessage(/*seq=*/3, /*handled=*/true);
}
+/**
+ * Once we send a resampled value to the app, we should continue to send the last predicted value if
+ * a pointer does not move. Only real values are used to determine if a pointer does not move.
+ */
+TEST_F(InputConsumerResamplingTest, ResampledValueIsUsedForIdenticalCoordinates) {
+ // Send the initial ACTION_DOWN separately, so that the first consumed event will only return an
+ // InputEvent with a single action.
+ mClientTestChannel->enqueueMessage(nextPointerMessage(
+ {0ms, {Pointer{.id = 0, .x = 10.0f, .y = 20.0f}}, AMOTION_EVENT_ACTION_DOWN}));
+
+ invokeLooperCallback();
+ assertReceivedMotionEvent({InputEventEntry{0ms,
+ {Pointer{.id = 0, .x = 10.0f, .y = 20.0f}},
+ AMOTION_EVENT_ACTION_DOWN}});
+
+ // Two ACTION_MOVE events 10 ms apart that move in X direction and stay still in Y
+ mClientTestChannel->enqueueMessage(nextPointerMessage(
+ {10ms, {Pointer{.id = 0, .x = 20.0f, .y = 30.0f}}, AMOTION_EVENT_ACTION_MOVE}));
+ mClientTestChannel->enqueueMessage(nextPointerMessage(
+ {20ms, {Pointer{.id = 0, .x = 30.0f, .y = 30.0f}}, AMOTION_EVENT_ACTION_MOVE}));
+
+ invokeLooperCallback();
+ mConsumer->consumeBatchedInputEvents(nanoseconds{35ms}.count());
+ assertReceivedMotionEvent(
+ {InputEventEntry{10ms,
+ {Pointer{.id = 0, .x = 20.0f, .y = 30.0f}},
+ AMOTION_EVENT_ACTION_MOVE},
+ InputEventEntry{20ms,
+ {Pointer{.id = 0, .x = 30.0f, .y = 30.0f}},
+ AMOTION_EVENT_ACTION_MOVE},
+ InputEventEntry{25ms,
+ {Pointer{.id = 0, .x = 35.0f, .y = 30.0f, .isResampled = true}},
+ AMOTION_EVENT_ACTION_MOVE}});
+
+ // Coordinate value 30 has been resampled to 35. When a new event comes in with value 30 again,
+ // the system should still report 35.
+ mClientTestChannel->enqueueMessage(nextPointerMessage(
+ {40ms, {Pointer{.id = 0, .x = 30.0f, .y = 30.0f}}, AMOTION_EVENT_ACTION_MOVE}));
+
+ invokeLooperCallback();
+ mConsumer->consumeBatchedInputEvents(nanoseconds{45ms + 5ms /*RESAMPLE_LATENCY*/}.count());
+ assertReceivedMotionEvent(
+ {InputEventEntry{40ms,
+ {Pointer{.id = 0, .x = 35.0f, .y = 30.0f, .isResampled = true}},
+ AMOTION_EVENT_ACTION_MOVE}, // original event, rewritten
+ InputEventEntry{45ms,
+ {Pointer{.id = 0, .x = 35.0f, .y = 30.0f, .isResampled = true}},
+ AMOTION_EVENT_ACTION_MOVE}}); // resampled event, rewritten
+
+ mClientTestChannel->assertFinishMessage(/*seq=*/1, /*handled=*/true);
+ mClientTestChannel->assertFinishMessage(/*seq=*/2, /*handled=*/true);
+ mClientTestChannel->assertFinishMessage(/*seq=*/3, /*handled=*/true);
+ mClientTestChannel->assertFinishMessage(/*seq=*/4, /*handled=*/true);
+}
+
} // namespace android