From e37f8346c62551dac1919fad4cd60a707d27e6a2 Mon Sep 17 00:00:00 2001 From: Paul Ramirez Date: Wed, 28 Aug 2024 18:42:21 +0000 Subject: Add multiple device resampling support to InputConsumerNoResampling with tests Added multiple device resampling support to InputConsumerNoResampling with unit tests to ensure correctness Bug: 297226446 Flag: EXEMPT refactor Test: TEST=libinput_tests; m $TEST && $ANDROID_HOST_OUT/nativetest64/$TEST/$TEST --gtest_filter="InputConsumerTest*" Change-Id: I45528a89e0b60f46b0095078356382ed701b191b --- include/input/InputConsumerNoResampling.h | 25 ++++++++++++++++++------- include/input/Resampler.h | 6 ------ 2 files changed, 18 insertions(+), 13 deletions(-) (limited to 'include/input') diff --git a/include/input/InputConsumerNoResampling.h b/include/input/InputConsumerNoResampling.h index c98b9cf8c1..228347d818 100644 --- a/include/input/InputConsumerNoResampling.h +++ b/include/input/InputConsumerNoResampling.h @@ -16,6 +16,7 @@ #pragma once +#include #include #include #include @@ -75,12 +76,13 @@ public: * the event is ready to consume. * @param looper needs to be sp and not shared_ptr because it inherits from * RefBase - * @param resampler the resampling strategy to use. If null, no resampling will be - * performed. + * @param resamplerCreator callable that returns the resampling strategy to be used. If null, no + * resampling will be performed. resamplerCreator must never return nullptr. */ - explicit InputConsumerNoResampling(const std::shared_ptr& channel, - sp looper, InputConsumerCallbacks& callbacks, - std::unique_ptr resampler); + explicit InputConsumerNoResampling( + const std::shared_ptr& channel, sp looper, + InputConsumerCallbacks& callbacks, + std::function()> resamplerCreator); ~InputConsumerNoResampling(); @@ -117,7 +119,13 @@ private: std::shared_ptr mChannel; sp mLooper; InputConsumerCallbacks& mCallbacks; - std::unique_ptr mResampler; + const std::function()> mResamplerCreator; + + /** + * A map to manage multidevice resampling. Each contained resampler is never null. This map is + * only modified by handleMessages. + */ + std::map> mResamplers; // Looper-related infrastructure /** @@ -190,7 +198,10 @@ private: /** * Batch messages that can be batched. When an unbatchable message is encountered, send it * to the InputConsumerCallbacks immediately. If there are batches remaining, - * notify InputConsumerCallbacks. + * notify InputConsumerCallbacks. If a resampleable ACTION_DOWN message is received, then a + * resampler is inserted for that deviceId in mResamplers. If a resampleable ACTION_UP or + * ACTION_CANCEL message is received then the resampler associated to that deviceId is erased + * from mResamplers. */ void handleMessages(std::vector&& messages); /** diff --git a/include/input/Resampler.h b/include/input/Resampler.h index dcb25b729f..4aaeddd159 100644 --- a/include/input/Resampler.h +++ b/include/input/Resampler.h @@ -91,12 +91,6 @@ private: } }; - /** - * Keeps track of the previous MotionEvent deviceId to enable comparison between the previous - * and the current deviceId. - */ - std::optional mPreviousDeviceId; - /** * Up to two latest samples from MotionEvent. Updated every time resampleMotionEvent is called. * Note: We store up to two samples in order to simplify the implementation. Although, -- cgit v1.2.3-59-g8ed1b From 8f87adef580c85fa5cfc4a68cd67e5df7ed83a7f Mon Sep 17 00:00:00 2001 From: Paul Ramirez Date: Tue, 1 Oct 2024 15:45:39 +0000 Subject: Add fields to InputMessageBuilder Added transformation fields to InputMessageBuilder. The purpose is to enable motion event methods when stubbing messages in tests. Bug: 297226446 Flag: EXEMPT refactor Test: TEST=libinput_tests; m $TEST && $ANDROID_HOST_OUT/nativetest64/$TEST/$TEST --gtest_filter="ResamplerTest*" Change-Id: Ic258d108d40287ef1d08751ab2f92d59e7e86f86 --- include/input/InputEventBuilders.h | 102 ++++++++++++++++++++++++++++++++++++- 1 file changed, 101 insertions(+), 1 deletion(-) (limited to 'include/input') diff --git a/include/input/InputEventBuilders.h b/include/input/InputEventBuilders.h index 5bd5070488..1899a66159 100644 --- a/include/input/InputEventBuilders.h +++ b/include/input/InputEventBuilders.h @@ -21,6 +21,7 @@ #include #include #include +#include #include // for nsecs_t, systemTime #include @@ -94,16 +95,81 @@ public: return *this; } + InputMessageBuilder& hmac(const std::array& hmac) { + mHmac = hmac; + return *this; + } + InputMessageBuilder& action(int32_t action) { mAction = action; return *this; } + InputMessageBuilder& actionButton(int32_t actionButton) { + mActionButton = actionButton; + return *this; + } + + InputMessageBuilder& flags(int32_t flags) { + mFlags = flags; + return *this; + } + + InputMessageBuilder& metaState(int32_t metaState) { + mMetaState = metaState; + return *this; + } + + InputMessageBuilder& buttonState(int32_t buttonState) { + mButtonState = buttonState; + return *this; + } + + InputMessageBuilder& classification(MotionClassification classification) { + mClassification = classification; + return *this; + } + + InputMessageBuilder& edgeFlags(int32_t edgeFlags) { + mEdgeFlags = edgeFlags; + return *this; + } + InputMessageBuilder& downTime(nsecs_t downTime) { mDownTime = downTime; return *this; } + InputMessageBuilder& transform(const ui::Transform& transform) { + mTransform = transform; + return *this; + } + + InputMessageBuilder& xPrecision(float xPrecision) { + mXPrecision = xPrecision; + return *this; + } + + InputMessageBuilder& yPrecision(float yPrecision) { + mYPrecision = yPrecision; + return *this; + } + + InputMessageBuilder& xCursorPosition(float xCursorPosition) { + mXCursorPosition = xCursorPosition; + return *this; + } + + InputMessageBuilder& yCursorPosition(float yCursorPosition) { + mYCursorPosition = yCursorPosition; + return *this; + } + + InputMessageBuilder& rawTransform(const ui::Transform& rawTransform) { + mRawTransform = rawTransform; + return *this; + } + InputMessageBuilder& pointer(PointerBuilder pointerBuilder) { mPointers.push_back(pointerBuilder); return *this; @@ -121,8 +187,30 @@ public: message.body.motion.deviceId = mDeviceId; message.body.motion.source = mSource; message.body.motion.displayId = mDisplayId.val(); + message.body.motion.hmac = std::move(mHmac); message.body.motion.action = mAction; + message.body.motion.actionButton = mActionButton; + message.body.motion.flags = mFlags; + message.body.motion.metaState = mMetaState; + message.body.motion.buttonState = mButtonState; + message.body.motion.edgeFlags = mEdgeFlags; message.body.motion.downTime = mDownTime; + message.body.motion.dsdx = mTransform.dsdx(); + message.body.motion.dtdx = mTransform.dtdx(); + message.body.motion.dtdy = mTransform.dtdy(); + message.body.motion.dsdy = mTransform.dsdy(); + message.body.motion.tx = mTransform.ty(); + message.body.motion.ty = mTransform.tx(); + message.body.motion.xPrecision = mXPrecision; + message.body.motion.yPrecision = mYPrecision; + message.body.motion.xCursorPosition = mXCursorPosition; + message.body.motion.yCursorPosition = mYCursorPosition; + message.body.motion.dsdxRaw = mRawTransform.dsdx(); + message.body.motion.dtdxRaw = mRawTransform.dtdx(); + message.body.motion.dtdyRaw = mRawTransform.dtdy(); + message.body.motion.dsdyRaw = mRawTransform.dsdy(); + message.body.motion.txRaw = mRawTransform.ty(); + message.body.motion.tyRaw = mRawTransform.tx(); for (size_t i = 0; i < mPointers.size(); ++i) { message.body.motion.pointers[i].properties = mPointers[i].buildProperties(); @@ -140,9 +228,21 @@ private: DeviceId mDeviceId{DEFAULT_DEVICE_ID}; int32_t mSource{AINPUT_SOURCE_TOUCHSCREEN}; ui::LogicalDisplayId mDisplayId{ui::LogicalDisplayId::DEFAULT}; + std::array mHmac{INVALID_HMAC}; int32_t mAction{AMOTION_EVENT_ACTION_MOVE}; + int32_t mActionButton{0}; + int32_t mFlags{0}; + int32_t mMetaState{AMETA_NONE}; + int32_t mButtonState{0}; + MotionClassification mClassification{MotionClassification::NONE}; + int32_t mEdgeFlags{0}; nsecs_t mDownTime{mEventTime}; - + ui::Transform mTransform{}; + float mXPrecision{1.0f}; + float mYPrecision{1.0f}; + float mXCursorPosition{AMOTION_EVENT_INVALID_CURSOR_POSITION}; + float mYCursorPosition{AMOTION_EVENT_INVALID_CURSOR_POSITION}; + ui::Transform mRawTransform{}; std::vector mPointers; }; -- cgit v1.2.3-59-g8ed1b From 7f1efed1f8fcae76c021bfcea244c4f92844a608 Mon Sep 17 00:00:00 2001 From: Paul Ramirez Date: Sun, 29 Sep 2024 23:55:23 +0000 Subject: Add check to not resample when resample time equals motion event time Included SampleTimeEqualsEventTime from TouchResampling_test.cpp into InputConsumerResampling_test.cpp, and added the missing logic in LegacyResampler to pass the test. Bug: 297226446 Flag: EXEMPT refactor Test: TEST=libinput_tests; m $TEST && $ANDROID_HOST_OUT/nativetest64/$TEST/$TEST --gtest_filter="InputConsumerResamplingTest*" Change-Id: I8ff9a263ea79eed1b814d2b1ce0b7efb5ade584e --- include/input/Resampler.h | 3 +- libs/input/Resampler.cpp | 5 +++ libs/input/tests/InputConsumerResampling_test.cpp | 51 +++++++++++++++++++---- 3 files changed, 51 insertions(+), 8 deletions(-) (limited to 'include/input') diff --git a/include/input/Resampler.h b/include/input/Resampler.h index 4aaeddd159..da0c5b2150 100644 --- a/include/input/Resampler.h +++ b/include/input/Resampler.h @@ -65,7 +65,8 @@ public: * extrapolation takes place and `resampleTime` is too far in the future. If `futureSample` is * not null, interpolation will occur. If `futureSample` is null and there is enough historical * data, LegacyResampler will extrapolate. Otherwise, no resampling takes place and - * `motionEvent` is unmodified. + * `motionEvent` is unmodified. Furthermore, motionEvent is not resampled if resampleTime equals + * the last sample eventTime of motionEvent. */ void resampleMotionEvent(std::chrono::nanoseconds frameTime, MotionEvent& motionEvent, const InputMessage* futureSample) override; diff --git a/libs/input/Resampler.cpp b/libs/input/Resampler.cpp index 328fa684f6..1adff7ba36 100644 --- a/libs/input/Resampler.cpp +++ b/libs/input/Resampler.cpp @@ -249,6 +249,11 @@ void LegacyResampler::resampleMotionEvent(nanoseconds frameTime, MotionEvent& mo const InputMessage* futureSample) { const nanoseconds resampleTime = frameTime - RESAMPLE_LATENCY; + if (resampleTime.count() == motionEvent.getEventTime()) { + LOG_IF(INFO, debugResampling()) << "Not resampled. Resample time equals motion event time."; + return; + } + updateLatestSamples(motionEvent); const std::optional sample = (futureSample != nullptr) diff --git a/libs/input/tests/InputConsumerResampling_test.cpp b/libs/input/tests/InputConsumerResampling_test.cpp index 61a0a98c9e..b139e8766f 100644 --- a/libs/input/tests/InputConsumerResampling_test.cpp +++ b/libs/input/tests/InputConsumerResampling_test.cpp @@ -1,4 +1,3 @@ - /* * Copyright (C) 2024 The Android Open Source Project * @@ -193,7 +192,7 @@ void InputConsumerResamplingTest::assertReceivedMotionEvent( * last two real events, which would put this time at: 20 ms + (20 ms - 10 ms) / 2 = 25 ms. */ TEST_F(InputConsumerResamplingTest, EventIsResampled) { - // Initial ACTION_DOWN should be separate, because the first consume event will only return + // 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})); @@ -234,7 +233,7 @@ TEST_F(InputConsumerResamplingTest, EventIsResampled) { * have these hardcoded. */ TEST_F(InputConsumerResamplingTest, EventIsResampledWithDifferentId) { - // Initial ACTION_DOWN should be separate, because the first consume event will only return + // 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 = 1, .x = 10.0f, .y = 20.0f}}, AMOTION_EVENT_ACTION_DOWN})); @@ -274,7 +273,7 @@ TEST_F(InputConsumerResamplingTest, EventIsResampledWithDifferentId) { * Stylus pointer coordinates are resampled. */ TEST_F(InputConsumerResamplingTest, StylusEventIsResampled) { - // Initial ACTION_DOWN should be separate, because the first consume event will only return + // 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, @@ -332,9 +331,8 @@ TEST_F(InputConsumerResamplingTest, StylusEventIsResampled) { * Mouse pointer coordinates are resampled. */ TEST_F(InputConsumerResamplingTest, MouseEventIsResampled) { - // Initial ACTION_DOWN should be separate, because the first consume event will only return + // 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, .toolType = ToolType::MOUSE}}, @@ -391,7 +389,7 @@ TEST_F(InputConsumerResamplingTest, MouseEventIsResampled) { * Motion events with palm tool type are not resampled. */ TEST_F(InputConsumerResamplingTest, PalmEventIsNotResampled) { - // Initial ACTION_DOWN should be separate, because the first consume event will only return + // 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, @@ -431,4 +429,43 @@ TEST_F(InputConsumerResamplingTest, PalmEventIsNotResampled) { mClientTestChannel->assertFinishMessage(/*seq=*/3, /*handled=*/true); } +/** + * Event should not be resampled when sample time is equal to event time. + */ +TEST_F(InputConsumerResamplingTest, SampleTimeEqualsEventTime) { + // 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})); + + mClientTestChannel->assertNoSentMessages(); + + 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{20ms + 5ms /*RESAMPLE_LATENCY*/}.count()); + + // MotionEvent should not resampled because the resample time falls exactly on the existing + // event time. + 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}}); + + mClientTestChannel->assertFinishMessage(/*seq=*/1, /*handled=*/true); + mClientTestChannel->assertFinishMessage(/*seq=*/2, /*handled=*/true); + mClientTestChannel->assertFinishMessage(/*seq=*/3, /*handled=*/true); +} + } // namespace android -- cgit v1.2.3-59-g8ed1b From 5a71e8857560729d165e3154c97788201e063ece Mon Sep 17 00:00:00 2001 From: Vaibhav Devmurari Date: Mon, 30 Sep 2024 15:54:41 +0000 Subject: hasKeycode API should take into account key remapping Keyboards can generate certain keycodes even if the keys are not present in the device HID descriptor, by using key remapping APIs Test: atest ModifierKeyRemappingTest Bug: 368397939 Flag: EXEMPT bugfix Change-Id: I30afda89f289eddc2b05fb124555aebfb182852e --- include/input/KeyCharacterMap.h | 3 +++ libs/input/KeyCharacterMap.cpp | 11 +++++++++++ services/inputflinger/reader/EventHub.cpp | 14 +++++++++++++- services/inputflinger/reader/include/EventHub.h | 1 + 4 files changed, 28 insertions(+), 1 deletion(-) (limited to 'include/input') diff --git a/include/input/KeyCharacterMap.h b/include/input/KeyCharacterMap.h index 67b37b1213..7ea34c2566 100644 --- a/include/input/KeyCharacterMap.h +++ b/include/input/KeyCharacterMap.h @@ -137,6 +137,9 @@ public: /* Returns keycode after applying Android key code remapping defined in mKeyRemapping */ int32_t applyKeyRemapping(int32_t fromKeyCode) const; + /** Returns list of keycodes that remap to provided keycode (@see setKeyRemapping()) */ + std::vector findKeyCodesMappedToKeyCode(int32_t toKeyCode) const; + /* Returns the pair after applying key behavior defined in the kcm file, * that tries to find a replacement key code based on current meta state */ std::pair applyKeyBehavior(int32_t keyCode, diff --git a/libs/input/KeyCharacterMap.cpp b/libs/input/KeyCharacterMap.cpp index b0563abaf7..d775327a80 100644 --- a/libs/input/KeyCharacterMap.cpp +++ b/libs/input/KeyCharacterMap.cpp @@ -365,6 +365,17 @@ int32_t KeyCharacterMap::applyKeyRemapping(int32_t fromKeyCode) const { return toKeyCode; } +std::vector KeyCharacterMap::findKeyCodesMappedToKeyCode(int32_t toKeyCode) const { + std::vector fromKeyCodes; + + for (const auto& [from, to] : mKeyRemapping) { + if (toKeyCode == to) { + fromKeyCodes.push_back(from); + } + } + return fromKeyCodes; +} + std::pair KeyCharacterMap::applyKeyBehavior(int32_t fromKeyCode, int32_t fromMetaState) const { int32_t toKeyCode = fromKeyCode; diff --git a/services/inputflinger/reader/EventHub.cpp b/services/inputflinger/reader/EventHub.cpp index 0865eed4a2..10cc1eed7b 100644 --- a/services/inputflinger/reader/EventHub.cpp +++ b/services/inputflinger/reader/EventHub.cpp @@ -659,6 +659,19 @@ void EventHub::Device::populateAbsoluteAxisStates() { } bool EventHub::Device::hasKeycodeLocked(int keycode) const { + if (hasKeycodeInternalLocked(keycode)) { + return true; + } + + for (auto& fromKey : getKeyCharacterMap()->findKeyCodesMappedToKeyCode(keycode)) { + if (hasKeycodeInternalLocked(fromKey)) { + return true; + } + } + return false; +} + +bool EventHub::Device::hasKeycodeInternalLocked(int keycode) const { if (!keyMap.haveKeyLayout()) { return false; } @@ -676,7 +689,6 @@ bool EventHub::Device::hasKeycodeLocked(int keycode) const { if (usageCodes.size() > 0 && mscBitmask.test(MSC_SCAN)) { return true; } - return false; } diff --git a/services/inputflinger/reader/include/EventHub.h b/services/inputflinger/reader/include/EventHub.h index edc30379b2..dffd8e3c7a 100644 --- a/services/inputflinger/reader/include/EventHub.h +++ b/services/inputflinger/reader/include/EventHub.h @@ -680,6 +680,7 @@ private: void configureFd(); void populateAbsoluteAxisStates(); bool hasKeycodeLocked(int keycode) const; + bool hasKeycodeInternalLocked(int keycode) const; void loadConfigurationLocked(); bool loadVirtualKeyMapLocked(); status_t loadKeyMapLocked(); -- cgit v1.2.3-59-g8ed1b From 4d3b03adfa3543158c982546f3d6daec7eed06e8 Mon Sep 17 00:00:00 2001 From: Paul Ramirez Date: Mon, 30 Sep 2024 01:39:00 +0000 Subject: 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 --- include/input/Resampler.h | 22 +++++++ libs/input/Resampler.cpp | 72 ++++++++++++++++++++--- libs/input/tests/InputConsumerResampling_test.cpp | 67 +++++++++++++++++---- 3 files changed, 140 insertions(+), 21 deletions(-) (limited to 'include/input') 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 @@ -99,6 +99,17 @@ private: */ RingBuffer mLatestSamples{/*capacity=*/2}; + /** + * Latest sample in mLatestSamples after resampling motion event. Used to compare if a pointer + * does not move between samples. + */ + std::optional mLastRealSample; + + /** + * Latest prediction. Used to overwrite motion event samples if a set of conditions is met. + */ + std::optional 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 @@ -144,6 +155,17 @@ private: */ std::optional 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 #include +#include #include #include @@ -26,10 +27,7 @@ #include #include -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( + *(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 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 -- cgit v1.2.3-59-g8ed1b From 4679e55027a36e63cfdc642de313cb2c46c58c54 Mon Sep 17 00:00:00 2001 From: Paul Ramirez Date: Tue, 1 Oct 2024 01:17:39 +0000 Subject: Add logic to overwrite pointer coordinates if event time is too old Included OldEventReceivedAfterResampleOccurs 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: I41cb79eaba965cfdfe7db68c388cb5d0ffa406f3 --- include/input/Resampler.h | 6 +++ libs/input/Resampler.cpp | 19 ++++++++ libs/input/tests/InputConsumerResampling_test.cpp | 55 +++++++++++++++++++++++ 3 files changed, 80 insertions(+) (limited to 'include/input') diff --git a/include/input/Resampler.h b/include/input/Resampler.h index f04dfde995..47519c2cfd 100644 --- a/include/input/Resampler.h +++ b/include/input/Resampler.h @@ -166,6 +166,12 @@ private: */ void overwriteStillPointers(MotionEvent& motionEvent, size_t sampleIndex) const; + /** + * Overwrites the pointer coordinates of a sample with event time older than + * that of mPreviousPrediction. + */ + void overwriteOldPointers(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 8fe904f9a4..e2cc6fb174 100644 --- a/libs/input/Resampler.cpp +++ b/libs/input/Resampler.cpp @@ -271,6 +271,7 @@ void LegacyResampler::overwriteMotionEventSamples(MotionEvent& motionEvent) cons const size_t numSamples = motionEvent.getHistorySize() + 1; for (size_t sampleIndex = 0; sampleIndex < numSamples; ++sampleIndex) { overwriteStillPointers(motionEvent, sampleIndex); + overwriteOldPointers(motionEvent, sampleIndex); } } @@ -289,6 +290,24 @@ void LegacyResampler::overwriteStillPointers(MotionEvent& motionEvent, size_t sa } } +void LegacyResampler::overwriteOldPointers(MotionEvent& motionEvent, size_t sampleIndex) const { + if (!mPreviousPrediction.has_value()) { + return; + } + if (nanoseconds{motionEvent.getHistoricalEventTime(sampleIndex)} < + mPreviousPrediction->eventTime) { + LOG_IF(INFO, debugResampling()) + << "Motion event sample older than predicted sample. Overwriting event time from " + << motionEvent.getHistoricalEventTime(sampleIndex) << "ns to " + << mPreviousPrediction->eventTime.count() << "ns."; + for (size_t pointerIndex = 0; pointerIndex < motionEvent.getPointerCount(); + ++pointerIndex) { + setMotionEventPointerCoords(motionEvent, sampleIndex, pointerIndex, + mPreviousPrediction->pointers[pointerIndex].coords); + } + } +} + void LegacyResampler::resampleMotionEvent(nanoseconds frameTime, MotionEvent& motionEvent, const InputMessage* futureSample) { const nanoseconds resampleTime = frameTime - RESAMPLE_LATENCY; diff --git a/libs/input/tests/InputConsumerResampling_test.cpp b/libs/input/tests/InputConsumerResampling_test.cpp index 85311afea9..883ca82fe0 100644 --- a/libs/input/tests/InputConsumerResampling_test.cpp +++ b/libs/input/tests/InputConsumerResampling_test.cpp @@ -511,4 +511,59 @@ TEST_F(InputConsumerResamplingTest, ResampledValueIsUsedForIdenticalCoordinates) mClientTestChannel->assertFinishMessage(/*seq=*/4, /*handled=*/true); } +TEST_F(InputConsumerResamplingTest, OldEventReceivedAfterResampleOccurs) { + // 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}}); + + // Above, the resampled event is at 25ms rather than at 30 ms = 35ms - RESAMPLE_LATENCY + // because we are further bound by how far we can extrapolate by the "last time delta". + // That's 50% of (20 ms - 10ms) => 5ms. So we can't predict more than 5 ms into the future + // from the event at 20ms, which is why the resampled event is at t = 25 ms. + + // We resampled the event to 25 ms. Now, an older 'real' event comes in. + mClientTestChannel->enqueueMessage(nextPointerMessage( + {24ms, {Pointer{.id = 0, .x = 40.0f, .y = 30.0f}}, AMOTION_EVENT_ACTION_MOVE})); + + invokeLooperCallback(); + mConsumer->consumeBatchedInputEvents(nanoseconds{50ms}.count()); + assertReceivedMotionEvent( + {InputEventEntry{24ms, + {Pointer{.id = 0, .x = 35.0f, .y = 30.0f, .isResampled = true}}, + AMOTION_EVENT_ACTION_MOVE}, // original event, rewritten + InputEventEntry{26ms, + {Pointer{.id = 0, .x = 45.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 -- cgit v1.2.3-59-g8ed1b From f6abbf4b919b121614384ab2286bcaf16bedd344 Mon Sep 17 00:00:00 2001 From: Siarhei Vishniakou Date: Mon, 7 Oct 2024 10:58:40 -0700 Subject: Reject inconsistent globally injected events The dispatcher currently crashes when certain inconsistent input events are injected. This is affecting test stability negatively. In this CL, we reject globally-injected inconsistent events. That may cause some test flakiness, but should eliminate the crashes due to dispatcher reaching bad state later. Unfortunately, we can't currently reject all inconsistent injected events. In the case of targeted injection, it is common for the caller to leave pointers dangling. Since the injection happens into the caller-owned windows only, at the end of those tests the windows get cleaned up, so the dispatcher is still in a good state. The eventual goal is to completely get rid of injection. Meanwhile, however, this should help avoid at least some of the crashes. Bug: 369935405 Flag: EXEMPT bugfix Test: atest inputflinger_tests Change-Id: I0696dbd3e4c5b88aad5aa853759227c0b56d5374 --- include/input/InputEventBuilders.h | 3 ++ .../inputflinger/dispatcher/InputDispatcher.cpp | 63 +++++++++++++--------- services/inputflinger/dispatcher/InputDispatcher.h | 4 ++ .../inputflinger/tests/InputDispatcher_test.cpp | 16 ++++++ 4 files changed, 60 insertions(+), 26 deletions(-) (limited to 'include/input') diff --git a/include/input/InputEventBuilders.h b/include/input/InputEventBuilders.h index 1899a66159..1696a62693 100644 --- a/include/input/InputEventBuilders.h +++ b/include/input/InputEventBuilders.h @@ -250,6 +250,9 @@ class MotionEventBuilder { public: MotionEventBuilder(int32_t action, int32_t source) { mAction = action; + if (mAction == AMOTION_EVENT_ACTION_CANCEL) { + mFlags |= AMOTION_EVENT_FLAG_CANCELED; + } mSource = source; mEventTime = systemTime(SYSTEM_TIME_MONOTONIC); mDownTime = mEventTime; diff --git a/services/inputflinger/dispatcher/InputDispatcher.cpp b/services/inputflinger/dispatcher/InputDispatcher.cpp index 4b43c277b6..f6182771fc 100644 --- a/services/inputflinger/dispatcher/InputDispatcher.cpp +++ b/services/inputflinger/dispatcher/InputDispatcher.cpp @@ -4796,6 +4796,39 @@ void InputDispatcher::notifyPointerCaptureChanged(const NotifyPointerCaptureChan } } +bool InputDispatcher::shouldRejectInjectedMotionLocked(const MotionEvent& motionEvent, + DeviceId deviceId, + ui::LogicalDisplayId displayId, + std::optional targetUid, + int32_t flags) { + // Don't verify targeted injection, since it will only affect the caller's + // window, and the windows are typically destroyed at the end of the test. + if (targetUid.has_value()) { + return false; + } + + // Verify all other injected streams, whether the injection is coming from apps or from + // input filter. Print an error if the stream becomes inconsistent with this event. + // An inconsistent injected event sent could cause a crash in the later stages of + // dispatching pipeline. + auto [it, _] = mInputFilterVerifiersByDisplay.try_emplace(displayId, + std::string("Injection on ") + + displayId.toString()); + InputVerifier& verifier = it->second; + + Result result = + verifier.processMovement(deviceId, motionEvent.getSource(), motionEvent.getAction(), + motionEvent.getPointerCount(), + motionEvent.getPointerProperties(), + motionEvent.getSamplePointerCoords(), flags); + if (!result.ok()) { + logDispatchStateLocked(); + LOG(ERROR) << "Inconsistent event: " << motionEvent << ", reason: " << result.error(); + return true; + } + return false; +} + InputEventInjectionResult InputDispatcher::injectInputEvent(const InputEvent* event, std::optional targetUid, InputEventInjectionSync syncMode, @@ -4906,32 +4939,10 @@ InputEventInjectionResult InputDispatcher::injectInputEvent(const InputEvent* ev mLock.lock(); - { - // Verify all injected streams, whether the injection is coming from apps or from - // input filter. Print an error if the stream becomes inconsistent with this event. - // An inconsistent injected event sent could cause a crash in the later stages of - // dispatching pipeline. - auto [it, _] = - mInputFilterVerifiersByDisplay.try_emplace(displayId, - std::string("Injection on ") + - displayId.toString()); - InputVerifier& verifier = it->second; - - Result result = - verifier.processMovement(resolvedDeviceId, motionEvent.getSource(), - motionEvent.getAction(), - motionEvent.getPointerCount(), - motionEvent.getPointerProperties(), - motionEvent.getSamplePointerCoords(), flags); - if (!result.ok()) { - logDispatchStateLocked(); - LOG(ERROR) << "Inconsistent event: " << motionEvent - << ", reason: " << result.error(); - if (policyFlags & POLICY_FLAG_INJECTED_FROM_ACCESSIBILITY) { - mLock.unlock(); - return InputEventInjectionResult::FAILED; - } - } + if (shouldRejectInjectedMotionLocked(motionEvent, resolvedDeviceId, displayId, + targetUid, flags)) { + mLock.unlock(); + return InputEventInjectionResult::FAILED; } const nsecs_t* sampleEventTimes = motionEvent.getSampleEventTimes(); diff --git a/services/inputflinger/dispatcher/InputDispatcher.h b/services/inputflinger/dispatcher/InputDispatcher.h index 24e36ae710..a6b7cc75c1 100644 --- a/services/inputflinger/dispatcher/InputDispatcher.h +++ b/services/inputflinger/dispatcher/InputDispatcher.h @@ -299,6 +299,10 @@ private: // Event injection and synchronization. std::condition_variable mInjectionResultAvailable; + bool shouldRejectInjectedMotionLocked(const MotionEvent& motion, DeviceId deviceId, + ui::LogicalDisplayId displayId, + std::optional targetUid, int32_t flags) + REQUIRES(mLock); void setInjectionResult(const EventEntry& entry, android::os::InputEventInjectionResult injectionResult); void transformMotionEntryForInjectionLocked(MotionEntry&, diff --git a/services/inputflinger/tests/InputDispatcher_test.cpp b/services/inputflinger/tests/InputDispatcher_test.cpp index c5702e92d9..df8bb99520 100644 --- a/services/inputflinger/tests/InputDispatcher_test.cpp +++ b/services/inputflinger/tests/InputDispatcher_test.cpp @@ -12889,6 +12889,22 @@ TEST_F(InputDispatcherDragTests, DragAndDropFinishedWhenCancelCurrentTouch) { // Remove drag window mDispatcher->onWindowInfosChanged({{*mWindow->getInfo(), *mSecondWindow->getInfo()}, {}, 0, 0}); + // Complete the first event stream, even though the injection will fail because there aren't any + // valid targets to dispatch this event to. This is still needed to make the input stream + // consistent + ASSERT_EQ(InputEventInjectionResult::FAILED, + injectMotionEvent(*mDispatcher, + MotionEventBuilder(ACTION_CANCEL, AINPUT_SOURCE_TOUCHSCREEN) + .displayId(ui::LogicalDisplayId::DEFAULT) + .pointer(PointerBuilder(/*id=*/0, ToolType::FINGER) + .x(150) + .y(50)) + .pointer(PointerBuilder(/*id=*/1, ToolType::FINGER) + .x(50) + .y(50)) + .build(), + INJECT_EVENT_TIMEOUT, InputEventInjectionSync::WAIT_FOR_RESULT)); + // Inject a simple gesture, ensure dispatcher not crashed ASSERT_EQ(InputEventInjectionResult::SUCCEEDED, injectMotionDown(*mDispatcher, AINPUT_SOURCE_TOUCHSCREEN, -- cgit v1.2.3-59-g8ed1b From f7f93f5fd5afa40858ac492402ec6cc85fe89275 Mon Sep 17 00:00:00 2001 From: "Liana Kazanova (xWF)" Date: Fri, 11 Oct 2024 22:19:28 +0000 Subject: Revert "Reject inconsistent globally injected events" This reverts commit f6abbf4b919b121614384ab2286bcaf16bedd344. Reason for revert: DroidMonitor: Potential culprit for http://b/372964183 - verifying through ABTD before revert submission. This is part of the standard investigation process, and does not mean your CL will be reverted Change-Id: I9cdb4dfb4ca14a90d7527e5f546380ea6ec1d62f --- include/input/InputEventBuilders.h | 3 -- .../inputflinger/dispatcher/InputDispatcher.cpp | 63 +++++++++------------- services/inputflinger/dispatcher/InputDispatcher.h | 4 -- .../inputflinger/tests/InputDispatcher_test.cpp | 16 ------ 4 files changed, 26 insertions(+), 60 deletions(-) (limited to 'include/input') diff --git a/include/input/InputEventBuilders.h b/include/input/InputEventBuilders.h index 1696a62693..1899a66159 100644 --- a/include/input/InputEventBuilders.h +++ b/include/input/InputEventBuilders.h @@ -250,9 +250,6 @@ class MotionEventBuilder { public: MotionEventBuilder(int32_t action, int32_t source) { mAction = action; - if (mAction == AMOTION_EVENT_ACTION_CANCEL) { - mFlags |= AMOTION_EVENT_FLAG_CANCELED; - } mSource = source; mEventTime = systemTime(SYSTEM_TIME_MONOTONIC); mDownTime = mEventTime; diff --git a/services/inputflinger/dispatcher/InputDispatcher.cpp b/services/inputflinger/dispatcher/InputDispatcher.cpp index f6182771fc..4b43c277b6 100644 --- a/services/inputflinger/dispatcher/InputDispatcher.cpp +++ b/services/inputflinger/dispatcher/InputDispatcher.cpp @@ -4796,39 +4796,6 @@ void InputDispatcher::notifyPointerCaptureChanged(const NotifyPointerCaptureChan } } -bool InputDispatcher::shouldRejectInjectedMotionLocked(const MotionEvent& motionEvent, - DeviceId deviceId, - ui::LogicalDisplayId displayId, - std::optional targetUid, - int32_t flags) { - // Don't verify targeted injection, since it will only affect the caller's - // window, and the windows are typically destroyed at the end of the test. - if (targetUid.has_value()) { - return false; - } - - // Verify all other injected streams, whether the injection is coming from apps or from - // input filter. Print an error if the stream becomes inconsistent with this event. - // An inconsistent injected event sent could cause a crash in the later stages of - // dispatching pipeline. - auto [it, _] = mInputFilterVerifiersByDisplay.try_emplace(displayId, - std::string("Injection on ") + - displayId.toString()); - InputVerifier& verifier = it->second; - - Result result = - verifier.processMovement(deviceId, motionEvent.getSource(), motionEvent.getAction(), - motionEvent.getPointerCount(), - motionEvent.getPointerProperties(), - motionEvent.getSamplePointerCoords(), flags); - if (!result.ok()) { - logDispatchStateLocked(); - LOG(ERROR) << "Inconsistent event: " << motionEvent << ", reason: " << result.error(); - return true; - } - return false; -} - InputEventInjectionResult InputDispatcher::injectInputEvent(const InputEvent* event, std::optional targetUid, InputEventInjectionSync syncMode, @@ -4939,10 +4906,32 @@ InputEventInjectionResult InputDispatcher::injectInputEvent(const InputEvent* ev mLock.lock(); - if (shouldRejectInjectedMotionLocked(motionEvent, resolvedDeviceId, displayId, - targetUid, flags)) { - mLock.unlock(); - return InputEventInjectionResult::FAILED; + { + // Verify all injected streams, whether the injection is coming from apps or from + // input filter. Print an error if the stream becomes inconsistent with this event. + // An inconsistent injected event sent could cause a crash in the later stages of + // dispatching pipeline. + auto [it, _] = + mInputFilterVerifiersByDisplay.try_emplace(displayId, + std::string("Injection on ") + + displayId.toString()); + InputVerifier& verifier = it->second; + + Result result = + verifier.processMovement(resolvedDeviceId, motionEvent.getSource(), + motionEvent.getAction(), + motionEvent.getPointerCount(), + motionEvent.getPointerProperties(), + motionEvent.getSamplePointerCoords(), flags); + if (!result.ok()) { + logDispatchStateLocked(); + LOG(ERROR) << "Inconsistent event: " << motionEvent + << ", reason: " << result.error(); + if (policyFlags & POLICY_FLAG_INJECTED_FROM_ACCESSIBILITY) { + mLock.unlock(); + return InputEventInjectionResult::FAILED; + } + } } const nsecs_t* sampleEventTimes = motionEvent.getSampleEventTimes(); diff --git a/services/inputflinger/dispatcher/InputDispatcher.h b/services/inputflinger/dispatcher/InputDispatcher.h index a6b7cc75c1..24e36ae710 100644 --- a/services/inputflinger/dispatcher/InputDispatcher.h +++ b/services/inputflinger/dispatcher/InputDispatcher.h @@ -299,10 +299,6 @@ private: // Event injection and synchronization. std::condition_variable mInjectionResultAvailable; - bool shouldRejectInjectedMotionLocked(const MotionEvent& motion, DeviceId deviceId, - ui::LogicalDisplayId displayId, - std::optional targetUid, int32_t flags) - REQUIRES(mLock); void setInjectionResult(const EventEntry& entry, android::os::InputEventInjectionResult injectionResult); void transformMotionEntryForInjectionLocked(MotionEntry&, diff --git a/services/inputflinger/tests/InputDispatcher_test.cpp b/services/inputflinger/tests/InputDispatcher_test.cpp index df8bb99520..c5702e92d9 100644 --- a/services/inputflinger/tests/InputDispatcher_test.cpp +++ b/services/inputflinger/tests/InputDispatcher_test.cpp @@ -12889,22 +12889,6 @@ TEST_F(InputDispatcherDragTests, DragAndDropFinishedWhenCancelCurrentTouch) { // Remove drag window mDispatcher->onWindowInfosChanged({{*mWindow->getInfo(), *mSecondWindow->getInfo()}, {}, 0, 0}); - // Complete the first event stream, even though the injection will fail because there aren't any - // valid targets to dispatch this event to. This is still needed to make the input stream - // consistent - ASSERT_EQ(InputEventInjectionResult::FAILED, - injectMotionEvent(*mDispatcher, - MotionEventBuilder(ACTION_CANCEL, AINPUT_SOURCE_TOUCHSCREEN) - .displayId(ui::LogicalDisplayId::DEFAULT) - .pointer(PointerBuilder(/*id=*/0, ToolType::FINGER) - .x(150) - .y(50)) - .pointer(PointerBuilder(/*id=*/1, ToolType::FINGER) - .x(50) - .y(50)) - .build(), - INJECT_EVENT_TIMEOUT, InputEventInjectionSync::WAIT_FOR_RESULT)); - // Inject a simple gesture, ensure dispatcher not crashed ASSERT_EQ(InputEventInjectionResult::SUCCEEDED, injectMotionDown(*mDispatcher, AINPUT_SOURCE_TOUCHSCREEN, -- cgit v1.2.3-59-g8ed1b From 67b101b5a69fc322e47806c7b1280ad3af556b7f Mon Sep 17 00:00:00 2001 From: Siarhei Vishniakou Date: Sat, 12 Oct 2024 00:42:44 +0000 Subject: Reland "Reject inconsistent globally injected events" --try 2 This reverts commit f7f93f5fd5afa40858ac492402ec6cc85fe89275. Reason for revert: fixed tests Changes: the test for inconsistent sequence needed to be updated because the injection will now always fail. Original description: The dispatcher currently crashes when certain inconsistent input events are injected. This is affecting test stability negatively. In this CL, we reject globally-injected inconsistent events. That may cause some test flakiness, but should eliminate the crashes due to dispatcher reaching bad state later. Unfortunately, we can't currently reject all inconsistent injected events. In the case of targeted injection, it is common for the caller to leave pointers dangling. Since the injection happens into the caller-owned windows only, at the end of those tests the windows get cleaned up, so the dispatcher is still in a good state. The eventual goal is to completely get rid of injection. Meanwhile, however, this should help avoid at least some of the crashes. Bug: 369935405 Flag: EXEMPT bugfix Test: atest inputflinger_tests Change-Id: Ic778677a63e544feadf01f3cc47b08ec3b207297 --- include/input/InputEventBuilders.h | 3 ++ .../inputflinger/dispatcher/InputDispatcher.cpp | 63 +++++++++++++--------- services/inputflinger/dispatcher/InputDispatcher.h | 4 ++ .../inputflinger/tests/InputDispatcher_test.cpp | 27 +++++++--- 4 files changed, 65 insertions(+), 32 deletions(-) (limited to 'include/input') diff --git a/include/input/InputEventBuilders.h b/include/input/InputEventBuilders.h index 1899a66159..1696a62693 100644 --- a/include/input/InputEventBuilders.h +++ b/include/input/InputEventBuilders.h @@ -250,6 +250,9 @@ class MotionEventBuilder { public: MotionEventBuilder(int32_t action, int32_t source) { mAction = action; + if (mAction == AMOTION_EVENT_ACTION_CANCEL) { + mFlags |= AMOTION_EVENT_FLAG_CANCELED; + } mSource = source; mEventTime = systemTime(SYSTEM_TIME_MONOTONIC); mDownTime = mEventTime; diff --git a/services/inputflinger/dispatcher/InputDispatcher.cpp b/services/inputflinger/dispatcher/InputDispatcher.cpp index 526dec6f9b..602904f562 100644 --- a/services/inputflinger/dispatcher/InputDispatcher.cpp +++ b/services/inputflinger/dispatcher/InputDispatcher.cpp @@ -4794,6 +4794,39 @@ void InputDispatcher::notifyPointerCaptureChanged(const NotifyPointerCaptureChan } } +bool InputDispatcher::shouldRejectInjectedMotionLocked(const MotionEvent& motionEvent, + DeviceId deviceId, + ui::LogicalDisplayId displayId, + std::optional targetUid, + int32_t flags) { + // Don't verify targeted injection, since it will only affect the caller's + // window, and the windows are typically destroyed at the end of the test. + if (targetUid.has_value()) { + return false; + } + + // Verify all other injected streams, whether the injection is coming from apps or from + // input filter. Print an error if the stream becomes inconsistent with this event. + // An inconsistent injected event sent could cause a crash in the later stages of + // dispatching pipeline. + auto [it, _] = mInputFilterVerifiersByDisplay.try_emplace(displayId, + std::string("Injection on ") + + displayId.toString()); + InputVerifier& verifier = it->second; + + Result result = + verifier.processMovement(deviceId, motionEvent.getSource(), motionEvent.getAction(), + motionEvent.getPointerCount(), + motionEvent.getPointerProperties(), + motionEvent.getSamplePointerCoords(), flags); + if (!result.ok()) { + logDispatchStateLocked(); + LOG(ERROR) << "Inconsistent event: " << motionEvent << ", reason: " << result.error(); + return true; + } + return false; +} + InputEventInjectionResult InputDispatcher::injectInputEvent(const InputEvent* event, std::optional targetUid, InputEventInjectionSync syncMode, @@ -4904,32 +4937,10 @@ InputEventInjectionResult InputDispatcher::injectInputEvent(const InputEvent* ev mLock.lock(); - { - // Verify all injected streams, whether the injection is coming from apps or from - // input filter. Print an error if the stream becomes inconsistent with this event. - // An inconsistent injected event sent could cause a crash in the later stages of - // dispatching pipeline. - auto [it, _] = - mInputFilterVerifiersByDisplay.try_emplace(displayId, - std::string("Injection on ") + - displayId.toString()); - InputVerifier& verifier = it->second; - - Result result = - verifier.processMovement(resolvedDeviceId, motionEvent.getSource(), - motionEvent.getAction(), - motionEvent.getPointerCount(), - motionEvent.getPointerProperties(), - motionEvent.getSamplePointerCoords(), flags); - if (!result.ok()) { - logDispatchStateLocked(); - LOG(ERROR) << "Inconsistent event: " << motionEvent - << ", reason: " << result.error(); - if (policyFlags & POLICY_FLAG_INJECTED_FROM_ACCESSIBILITY) { - mLock.unlock(); - return InputEventInjectionResult::FAILED; - } - } + if (shouldRejectInjectedMotionLocked(motionEvent, resolvedDeviceId, displayId, + targetUid, flags)) { + mLock.unlock(); + return InputEventInjectionResult::FAILED; } const nsecs_t* sampleEventTimes = motionEvent.getSampleEventTimes(); diff --git a/services/inputflinger/dispatcher/InputDispatcher.h b/services/inputflinger/dispatcher/InputDispatcher.h index 78cbf1eea4..fade8535c3 100644 --- a/services/inputflinger/dispatcher/InputDispatcher.h +++ b/services/inputflinger/dispatcher/InputDispatcher.h @@ -299,6 +299,10 @@ private: // Event injection and synchronization. std::condition_variable mInjectionResultAvailable; + bool shouldRejectInjectedMotionLocked(const MotionEvent& motion, DeviceId deviceId, + ui::LogicalDisplayId displayId, + std::optional targetUid, int32_t flags) + REQUIRES(mLock); void setInjectionResult(const EventEntry& entry, android::os::InputEventInjectionResult injectionResult); void transformMotionEntryForInjectionLocked(MotionEntry&, diff --git a/services/inputflinger/tests/InputDispatcher_test.cpp b/services/inputflinger/tests/InputDispatcher_test.cpp index 7b5c47b1ac..3413caa1f0 100644 --- a/services/inputflinger/tests/InputDispatcher_test.cpp +++ b/services/inputflinger/tests/InputDispatcher_test.cpp @@ -5105,9 +5105,7 @@ TEST_F(InputDispatcherTest, HoverExitIsSentToRemovedWindow) { /** * Test that invalid HOVER events sent by accessibility do not cause a fatal crash. */ -TEST_F_WITH_FLAGS(InputDispatcherTest, InvalidA11yHoverStreamDoesNotCrash, - REQUIRES_FLAGS_DISABLED(ACONFIG_FLAG(com::android::input::flags, - a11y_crash_on_inconsistent_event_stream))) { +TEST_F(InputDispatcherTest, InvalidA11yHoverStreamDoesNotCrash) { std::shared_ptr application = std::make_shared(); sp window = sp::make(application, mDispatcher, "Window", ui::LogicalDisplayId::DEFAULT); @@ -5123,10 +5121,11 @@ TEST_F_WITH_FLAGS(InputDispatcherTest, InvalidA11yHoverStreamDoesNotCrash, .addFlag(AMOTION_EVENT_FLAG_IS_ACCESSIBILITY_EVENT); ASSERT_EQ(InputEventInjectionResult::SUCCEEDED, injectMotionEvent(*mDispatcher, hoverEnterBuilder.build())); - ASSERT_EQ(InputEventInjectionResult::SUCCEEDED, - injectMotionEvent(*mDispatcher, hoverEnterBuilder.build())); - window->consumeMotionEvent(WithMotionAction(AMOTION_EVENT_ACTION_HOVER_ENTER)); window->consumeMotionEvent(WithMotionAction(AMOTION_EVENT_ACTION_HOVER_ENTER)); + // Another HOVER_ENTER would be inconsistent, and should therefore fail to + // get injected. + ASSERT_EQ(InputEventInjectionResult::FAILED, + injectMotionEvent(*mDispatcher, hoverEnterBuilder.build())); } /** @@ -12889,6 +12888,22 @@ TEST_F(InputDispatcherDragTests, DragAndDropFinishedWhenCancelCurrentTouch) { // Remove drag window mDispatcher->onWindowInfosChanged({{*mWindow->getInfo(), *mSecondWindow->getInfo()}, {}, 0, 0}); + // Complete the first event stream, even though the injection will fail because there aren't any + // valid targets to dispatch this event to. This is still needed to make the input stream + // consistent + ASSERT_EQ(InputEventInjectionResult::FAILED, + injectMotionEvent(*mDispatcher, + MotionEventBuilder(ACTION_CANCEL, AINPUT_SOURCE_TOUCHSCREEN) + .displayId(ui::LogicalDisplayId::DEFAULT) + .pointer(PointerBuilder(/*id=*/0, ToolType::FINGER) + .x(150) + .y(50)) + .pointer(PointerBuilder(/*id=*/1, ToolType::FINGER) + .x(50) + .y(50)) + .build(), + INJECT_EVENT_TIMEOUT, InputEventInjectionSync::WAIT_FOR_RESULT)); + // Inject a simple gesture, ensure dispatcher not crashed ASSERT_EQ(InputEventInjectionResult::SUCCEEDED, injectMotionDown(*mDispatcher, AINPUT_SOURCE_TOUCHSCREEN, -- cgit v1.2.3-59-g8ed1b From d78dd9b184cc700b9a887820056e5ed6a4c0ea56 Mon Sep 17 00:00:00 2001 From: Linnan Li Date: Tue, 15 Oct 2024 15:50:01 +0000 Subject: Copy KeyCharacterMap object when we fill InputDeviceInfo(1/n) Currently, in InputDeviceInfo, we store the KeyCharacterMap object, which is actually the original KeyCharacterMap from the EventHub. This could potentially lead to issues where two threads operate on the same KeyCharacterMap object simultaneously, resulting in thread safety problems. To avoid potential risks in the future, we make a copy of the original KeyCharacterMap when generating InputDeviceInfo. This change should not introduce any behavioral changes. Bug: 373011069 Flag: EXEMPT refactor Test: presubmit Signed-off-by: Linnan Li (cherry picked from https://partner-android-review.googlesource.com/q/commit:1be9c1ce7400e38cf3f45921d7181e947929f91c) Merged-In: I0d0155133f95b0f1dc925422eda0da04c6f196ea Change-Id: I0d0155133f95b0f1dc925422eda0da04c6f196ea --- include/input/InputDevice.h | 11 ++++---- include/input/KeyCharacterMap.h | 2 +- libs/input/InputDevice.cpp | 32 +++++++++++++++++++++- libs/input/KeyCharacterMap.cpp | 6 ++-- .../reader/mapper/KeyboardInputMapper.cpp | 4 ++- 5 files changed, 43 insertions(+), 12 deletions(-) (limited to 'include/input') diff --git a/include/input/InputDevice.h b/include/input/InputDevice.h index 1a482396ee..6a248ef188 100644 --- a/include/input/InputDevice.h +++ b/include/input/InputDevice.h @@ -266,6 +266,7 @@ class InputDeviceInfo { public: InputDeviceInfo(); InputDeviceInfo(const InputDeviceInfo& other); + InputDeviceInfo& operator=(const InputDeviceInfo& other); ~InputDeviceInfo(); struct MotionRange { @@ -315,13 +316,11 @@ public: inline const InputDeviceViewBehavior& getViewBehavior() const { return mViewBehavior; } - inline void setKeyCharacterMap(const std::shared_ptr value) { - mKeyCharacterMap = value; + inline void setKeyCharacterMap(std::unique_ptr value) { + mKeyCharacterMap = std::move(value); } - inline const std::shared_ptr getKeyCharacterMap() const { - return mKeyCharacterMap; - } + inline const KeyCharacterMap* getKeyCharacterMap() const { return mKeyCharacterMap.get(); } inline void setVibrator(bool hasVibrator) { mHasVibrator = hasVibrator; } inline bool hasVibrator() const { return mHasVibrator; } @@ -364,7 +363,7 @@ private: std::optional mKeyboardLayoutInfo; uint32_t mSources; int32_t mKeyboardType; - std::shared_ptr mKeyCharacterMap; + std::unique_ptr mKeyCharacterMap; std::optional mUsiVersion; ui::LogicalDisplayId mAssociatedDisplayId{ui::LogicalDisplayId::INVALID}; bool mEnabled; diff --git a/include/input/KeyCharacterMap.h b/include/input/KeyCharacterMap.h index 7ea34c2566..0a9e74f73b 100644 --- a/include/input/KeyCharacterMap.h +++ b/include/input/KeyCharacterMap.h @@ -72,7 +72,7 @@ public: }; /* Loads a key character map from a file. */ - static base::Result> load(const std::string& filename, + static base::Result> load(const std::string& filename, Format format); /* Loads a key character map from its string contents. */ diff --git a/libs/input/InputDevice.cpp b/libs/input/InputDevice.cpp index c9030312f9..4a6f66e058 100644 --- a/libs/input/InputDevice.cpp +++ b/libs/input/InputDevice.cpp @@ -191,7 +191,9 @@ InputDeviceInfo::InputDeviceInfo(const InputDeviceInfo& other) mKeyboardLayoutInfo(other.mKeyboardLayoutInfo), mSources(other.mSources), mKeyboardType(other.mKeyboardType), - mKeyCharacterMap(other.mKeyCharacterMap), + mKeyCharacterMap(other.mKeyCharacterMap + ? std::make_unique(*other.mKeyCharacterMap) + : nullptr), mUsiVersion(other.mUsiVersion), mAssociatedDisplayId(other.mAssociatedDisplayId), mEnabled(other.mEnabled), @@ -204,6 +206,34 @@ InputDeviceInfo::InputDeviceInfo(const InputDeviceInfo& other) mLights(other.mLights), mViewBehavior(other.mViewBehavior) {} +InputDeviceInfo& InputDeviceInfo::operator=(const InputDeviceInfo& other) { + mId = other.mId; + mGeneration = other.mGeneration; + mControllerNumber = other.mControllerNumber; + mIdentifier = other.mIdentifier; + mAlias = other.mAlias; + mIsExternal = other.mIsExternal; + mHasMic = other.mHasMic; + mKeyboardLayoutInfo = other.mKeyboardLayoutInfo; + mSources = other.mSources; + mKeyboardType = other.mKeyboardType; + mKeyCharacterMap = other.mKeyCharacterMap + ? std::make_unique(*other.mKeyCharacterMap) + : nullptr; + mUsiVersion = other.mUsiVersion; + mAssociatedDisplayId = other.mAssociatedDisplayId; + mEnabled = other.mEnabled; + mHasVibrator = other.mHasVibrator; + mHasBattery = other.mHasBattery; + mHasButtonUnderPad = other.mHasButtonUnderPad; + mHasSensor = other.mHasSensor; + mMotionRanges = other.mMotionRanges; + mSensors = other.mSensors; + mLights = other.mLights; + mViewBehavior = other.mViewBehavior; + return *this; +} + InputDeviceInfo::~InputDeviceInfo() { } diff --git a/libs/input/KeyCharacterMap.cpp b/libs/input/KeyCharacterMap.cpp index d775327a80..90d29dd190 100644 --- a/libs/input/KeyCharacterMap.cpp +++ b/libs/input/KeyCharacterMap.cpp @@ -84,15 +84,15 @@ static String8 toString(const char16_t* chars, size_t numChars) { KeyCharacterMap::KeyCharacterMap(const std::string& filename) : mLoadFileName(filename) {} -base::Result> KeyCharacterMap::load(const std::string& filename, +base::Result> KeyCharacterMap::load(const std::string& filename, Format format) { Tokenizer* tokenizer; status_t status = Tokenizer::open(String8(filename.c_str()), &tokenizer); if (status) { return Errorf("Error {} opening key character map file {}.", status, filename.c_str()); } - std::shared_ptr map = - std::shared_ptr(new KeyCharacterMap(filename)); + std::unique_ptr map = + std::unique_ptr(new KeyCharacterMap(filename)); if (!map.get()) { ALOGE("Error allocating key character map."); return Errorf("Error allocating key character map."); diff --git a/services/inputflinger/reader/mapper/KeyboardInputMapper.cpp b/services/inputflinger/reader/mapper/KeyboardInputMapper.cpp index 567a3e2ce4..585f61a3eb 100644 --- a/services/inputflinger/reader/mapper/KeyboardInputMapper.cpp +++ b/services/inputflinger/reader/mapper/KeyboardInputMapper.cpp @@ -132,7 +132,9 @@ std::optional KeyboardInputMapper::getKeyboardLayoutInfo() c void KeyboardInputMapper::populateDeviceInfo(InputDeviceInfo& info) { InputMapper::populateDeviceInfo(info); - info.setKeyCharacterMap(getDeviceContext().getKeyCharacterMap()); + if (const auto kcm = getDeviceContext().getKeyCharacterMap(); kcm != nullptr) { + info.setKeyCharacterMap(std::make_unique(*kcm)); + } std::optional keyboardLayoutInfo = getKeyboardLayoutInfo(); if (keyboardLayoutInfo) { -- cgit v1.2.3-59-g8ed1b From 362fa2273d66fae4af1ec38cac32c0ad22e1acf5 Mon Sep 17 00:00:00 2001 From: Siarhei Vishniakou Date: Thu, 17 Oct 2024 14:19:52 -0700 Subject: Remove callback at the end of consumer destructor It turns out that 'finishInputEvent' was adding the fd back to the looper. This caused an NPE because we were calling 'finishInputEvent' at the end of the consumer destructor, thus leaving the callback in the looper. The looper would hit an NPE whenever the fd signaled after that. To fix this, we move the call to setFdEvents(0) to the very end of the destructor. This way, we can be sure that the last thing that executes is the removal of the fd from the Looper. There is also a possibility that fd is signaled during the destructor execution. Theoretically, this should be okay because the fd callbacks are processed on the same thread as the one where destructor runs. Therefore, the signals would only be processed after the destructor has completed, which means the callback would be removed before it gets the chance to execute. Bug: 332613662 Test: TEST=libinput_tests; m $TEST && $ANDROID_HOST_OUT/nativetest64/$TEST/$TEST --gtest_filter="*InputConsumerTest*" Test: TEST=libutils_test; m $TEST && $ANDROID_HOST_OUT/nativetest64/$TEST/$TEST Flag: EXEMPT bugfix Change-Id: If5ac7a8eaf96e842d5d8e44008b9c1bff74e674e --- include/input/InputConsumerNoResampling.h | 2 +- libs/input/InputConsumerNoResampling.cpp | 17 +++++++++------ libs/input/tests/InputConsumer_test.cpp | 36 ++++++++++++++++++++++++++++--- 3 files changed, 44 insertions(+), 11 deletions(-) (limited to 'include/input') diff --git a/include/input/InputConsumerNoResampling.h b/include/input/InputConsumerNoResampling.h index 228347d818..2d1714c70a 100644 --- a/include/input/InputConsumerNoResampling.h +++ b/include/input/InputConsumerNoResampling.h @@ -141,7 +141,7 @@ private: } private: - std::function mCallback; + const std::function mCallback; }; sp mCallback; /** diff --git a/libs/input/InputConsumerNoResampling.cpp b/libs/input/InputConsumerNoResampling.cpp index 9665de799f..4cb6bf7bb4 100644 --- a/libs/input/InputConsumerNoResampling.cpp +++ b/libs/input/InputConsumerNoResampling.cpp @@ -193,13 +193,6 @@ InputConsumerNoResampling::InputConsumerNoResampling( InputConsumerNoResampling::~InputConsumerNoResampling() { ensureCalledOnLooperThread(__func__); - while (!mOutboundQueue.empty()) { - processOutboundEvents(); - // This is our last chance to ack the events. If we don't ack them here, we will get an ANR, - // so keep trying to send the events as long as they are present in the queue. - } - - setFdEvents(0); // If there are any remaining unread batches, send an ack for them and don't deliver // them to callbacks. for (auto& [_, batches] : mBatches) { @@ -208,6 +201,12 @@ InputConsumerNoResampling::~InputConsumerNoResampling() { batches.pop(); } } + + while (!mOutboundQueue.empty()) { + processOutboundEvents(); + // This is our last chance to ack the events. If we don't ack them here, we will get an ANR, + // so keep trying to send the events as long as they are present in the queue. + } // However, it is still up to the app to finish any events that have already been delivered // to the callbacks. If we wanted to change that behaviour and auto-finish all unfinished events // that were already sent to callbacks, we could potentially loop through "mConsumeTimes" @@ -216,6 +215,10 @@ InputConsumerNoResampling::~InputConsumerNoResampling() { const size_t unfinishedEvents = mConsumeTimes.size(); LOG_IF(INFO, unfinishedEvents != 0) << getName() << " has " << unfinishedEvents << " unfinished event(s)"; + // Remove the fd from epoll, so that Looper does not call 'handleReceiveCallback' anymore. + // This must be done at the end of the destructor; otherwise, some of the other functions may + // call 'setFdEvents' as a side-effect, thus adding the fd back to the epoll set of the looper. + setFdEvents(0); } int InputConsumerNoResampling::handleReceiveCallback(int events) { diff --git a/libs/input/tests/InputConsumer_test.cpp b/libs/input/tests/InputConsumer_test.cpp index 6a3bbe5b36..b32c2cb655 100644 --- a/libs/input/tests/InputConsumer_test.cpp +++ b/libs/input/tests/InputConsumer_test.cpp @@ -70,11 +70,20 @@ protected: []() { return std::make_unique(); }); } - void invokeLooperCallback() const { + bool invokeLooperCallback() const { sp callback; - ASSERT_TRUE(mLooper->getFdStateDebug(mClientTestChannel->getFd(), /*ident=*/nullptr, - /*events=*/nullptr, &callback, /*data=*/nullptr)); + const bool found = + mLooper->getFdStateDebug(mClientTestChannel->getFd(), /*ident=*/nullptr, + /*events=*/nullptr, &callback, /*data=*/nullptr); + if (!found) { + return false; + } + if (callback == nullptr) { + LOG(FATAL) << "Looper has the fd of interest, but the callback is null!"; + return false; + } callback->handleEvent(mClientTestChannel->getFd(), ALOOPER_EVENT_INPUT, /*data=*/nullptr); + return true; } void assertOnBatchedInputEventPendingWasCalled() { @@ -270,6 +279,27 @@ TEST_F(InputConsumerTest, UnhandledEventsNotFinishedInDestructor) { mClientTestChannel->assertNoSentMessages(); } +/** + * Check what happens when looper invokes callback after consumer has been destroyed. + * This reproduces a crash where the LooperEventCallback was added back to the Looper during + * destructor, thus allowing the looper callback to be invoked onto a null consumer object. + */ +TEST_F(InputConsumerTest, LooperCallbackInvokedAfterConsumerDestroyed) { + mClientTestChannel->enqueueMessage( + InputMessageBuilder{InputMessage::Type::MOTION, /*seq=*/0}.action(ACTION_DOWN).build()); + mClientTestChannel->enqueueMessage( + InputMessageBuilder{InputMessage::Type::MOTION, /*seq=*/1}.action(ACTION_MOVE).build()); + ASSERT_TRUE(invokeLooperCallback()); + assertOnBatchedInputEventPendingWasCalled(); + assertReceivedMotionEvent(WithMotionAction(ACTION_DOWN)); + mClientTestChannel->assertFinishMessage(/*seq=*/0, /*handled=*/true); + + // Now, destroy the consumer and invoke the looper callback again after it's been destroyed. + mConsumer.reset(); + mClientTestChannel->assertFinishMessage(/*seq=*/1, /*handled=*/false); + ASSERT_FALSE(invokeLooperCallback()); +} + /** * Send an event to the InputConsumer, but do not invoke "consumeBatchedInputEvents", thus leaving * the input event unconsumed by the callbacks. Ensure that no crash occurs when the consumer is -- cgit v1.2.3-59-g8ed1b From 29ee27c5a4d793c6d46f272c7704c1839334f71f Mon Sep 17 00:00:00 2001 From: Paul Ramirez Date: Wed, 2 Oct 2024 08:18:53 +0000 Subject: Add map like pointer lookup to LegacyResampler Added map lookup operations to find pointers by ID when resampling them inside LegacyResampler. The CL fixes the assumption that pointers should appear in the same order between motion events. Moreover, tests from LegacyResampler are updated to the new behavior. Bug: 297226446 Flag: EXEMPT refactor Test: TEST=libinput_tests; m $TEST && $ANDROID_HOST_OUT/nativetest64/$TEST/$TEST Change-Id: I59c83d2e55bf4ab3cb7958cab333428499f6fee8 --- include/input/Resampler.h | 135 +++++++++++++++++++++++++++++++++--- libs/input/Resampler.cpp | 118 +++++++++++++++++++------------ libs/input/tests/Resampler_test.cpp | 20 +++++- 3 files changed, 219 insertions(+), 54 deletions(-) (limited to 'include/input') diff --git a/include/input/Resampler.h b/include/input/Resampler.h index 47519c2cfd..6d95ca7e86 100644 --- a/include/input/Resampler.h +++ b/include/input/Resampler.h @@ -16,10 +16,14 @@ #pragma once +#include #include +#include #include #include +#include +#include #include #include #include @@ -79,13 +83,127 @@ private: PointerCoords coords; }; + /** + * Container that stores pointers as an associative array, supporting O(1) lookup by pointer id, + * as well as forward iteration in the order in which the pointer or pointers were inserted in + * the container. PointerMap has a maximum capacity equal to MAX_POINTERS. + */ + class PointerMap { + public: + struct PointerId : ftl::DefaultConstructible, + ftl::Equatable { + using DefaultConstructible::DefaultConstructible; + }; + + /** + * Custom iterator to enable use of range-based for loops. + */ + template + class iterator { + public: + using iterator_category = std::forward_iterator_tag; + using value_type = T; + using difference_type = std::ptrdiff_t; + using pointer = T*; + using reference = T&; + + explicit iterator(pointer element) : mElement{element} {} + + friend bool operator==(const iterator& lhs, const iterator& rhs) { + return lhs.mElement == rhs.mElement; + } + + friend bool operator!=(const iterator& lhs, const iterator& rhs) { + return !(lhs == rhs); + } + + iterator operator++() { + ++mElement; + return *this; + } + + reference operator*() const { return *mElement; } + + private: + pointer mElement; + }; + + PointerMap() { + idToIndex.fill(std::nullopt); + for (Pointer& pointer : pointers) { + pointer.properties.clear(); + pointer.coords.clear(); + } + } + + /** + * Forward iterators to traverse the pointers in `pointers`. The order of the pointers is + * determined by the order in which they were inserted (not by id). + */ + iterator begin() { return iterator{&pointers[0]}; } + + iterator begin() const { return iterator{&pointers[0]}; } + + iterator end() { return iterator{&pointers[nextPointerIndex]}; } + + iterator end() const { + return iterator{&pointers[nextPointerIndex]}; + } + + /** + * Inserts the given pointer into the PointerMap. Precondition: The current number of + * contained pointers must be less than MAX_POINTERS when this function is called. It + * fatally logs if the user tries to insert more than MAX_POINTERS, or if pointer id is out + * of bounds. + */ + void insert(const Pointer& pointer) { + LOG_IF(FATAL, nextPointerIndex >= pointers.size()) + << "Cannot insert more than " << MAX_POINTERS << " in PointerMap."; + LOG_IF(FATAL, (pointer.properties.id < 0) || (pointer.properties.id > MAX_POINTER_ID)) + << "Invalid pointer id."; + idToIndex[pointer.properties.id] = std::optional{nextPointerIndex}; + pointers[nextPointerIndex] = pointer; + ++nextPointerIndex; + } + + /** + * Returns the pointer associated with the provided id if it exists. + * Otherwise, std::nullopt is returned. + */ + std::optional find(PointerId id) const { + const int32_t idValue = ftl::to_underlying(id); + LOG_IF(FATAL, (idValue < 0) || (idValue > MAX_POINTER_ID)) << "Invalid pointer id."; + const std::optional index = idToIndex[idValue]; + return index.has_value() ? std::optional{pointers[*index]} : std::nullopt; + } + + private: + /** + * The index at which a pointer is inserted in `pointers`. Likewise, it represents the + * number of pointers in PointerMap. + */ + size_t nextPointerIndex{0}; + + /** + * Sequentially stores pointers. Each pointer's position is determined by the value of + * nextPointerIndex at insertion time. + */ + std::array pointers; + + /** + * Maps each pointer id to its associated index in pointers. If no pointer with the id + * exists in pointers, the mapped value is std::nullopt. + */ + std::array, MAX_POINTER_ID + 1> idToIndex; + }; + struct Sample { std::chrono::nanoseconds eventTime; - std::vector pointers; + PointerMap pointerMap; std::vector asPointerCoords() const { std::vector pointersCoords; - for (const Pointer& pointer : pointers) { + for (const Pointer& pointer : pointerMap) { pointersCoords.push_back(pointer.coords); } return pointersCoords; @@ -100,13 +218,12 @@ private: RingBuffer mLatestSamples{/*capacity=*/2}; /** - * Latest sample in mLatestSamples after resampling motion event. Used to compare if a pointer - * does not move between samples. + * Latest sample in mLatestSamples after resampling motion event. */ std::optional mLastRealSample; /** - * Latest prediction. Used to overwrite motion event samples if a set of conditions is met. + * Latest prediction. That is, the latest extrapolated sample. */ std::optional mPreviousPrediction; @@ -134,12 +251,12 @@ private: bool canInterpolate(const InputMessage& futureSample) const; /** - * Returns a sample interpolated between the latest sample of mLatestSamples and futureSample, + * Returns a sample interpolated between the latest sample of mLatestSamples and futureMessage, * if the conditions from canInterpolate are satisfied. Otherwise, returns nullopt. * mLatestSamples must have at least one sample when attemptInterpolation is called. */ std::optional attemptInterpolation(std::chrono::nanoseconds resampleTime, - const InputMessage& futureSample) const; + const InputMessage& futureMessage) const; /** * Checks if there are necessary conditions to extrapolate. That is, there are at least two @@ -156,7 +273,8 @@ private: std::optional attemptExtrapolation(std::chrono::nanoseconds resampleTime) const; /** - * Iterates through motion event samples, and calls overwriteStillPointers on each sample. + * Iterates through motion event samples, and replaces real coordinates with resampled + * coordinates to avoid jerkiness in certain conditions. */ void overwriteMotionEventSamples(MotionEvent& motionEvent) const; @@ -174,4 +292,5 @@ private: inline static void addSampleToMotionEvent(const Sample& sample, MotionEvent& motionEvent); }; + } // namespace android diff --git a/libs/input/Resampler.cpp b/libs/input/Resampler.cpp index e2cc6fb174..884b66e482 100644 --- a/libs/input/Resampler.cpp +++ b/libs/input/Resampler.cpp @@ -107,46 +107,44 @@ void LegacyResampler::updateLatestSamples(const MotionEvent& motionEvent) { const size_t latestIndex = numSamples - 1; const size_t secondToLatestIndex = (latestIndex > 0) ? (latestIndex - 1) : 0; for (size_t sampleIndex = secondToLatestIndex; sampleIndex < numSamples; ++sampleIndex) { - std::vector pointers; - const size_t numPointers = motionEvent.getPointerCount(); - for (size_t pointerIndex = 0; pointerIndex < numPointers; ++pointerIndex) { - pointers.push_back(Pointer{*(motionEvent.getPointerProperties(pointerIndex)), - *(motionEvent.getHistoricalRawPointerCoords(pointerIndex, - sampleIndex))}); + PointerMap pointerMap; + for (size_t pointerIndex = 0; pointerIndex < motionEvent.getPointerCount(); + ++pointerIndex) { + pointerMap.insert(Pointer{*(motionEvent.getPointerProperties(pointerIndex)), + *(motionEvent.getHistoricalRawPointerCoords(pointerIndex, + sampleIndex))}); } mLatestSamples.pushBack( - Sample{nanoseconds{motionEvent.getHistoricalEventTime(sampleIndex)}, pointers}); + Sample{nanoseconds{motionEvent.getHistoricalEventTime(sampleIndex)}, pointerMap}); } } LegacyResampler::Sample LegacyResampler::messageToSample(const InputMessage& message) { - std::vector pointers; + PointerMap pointerMap; for (uint32_t i = 0; i < message.body.motion.pointerCount; ++i) { - pointers.push_back(Pointer{message.body.motion.pointers[i].properties, - message.body.motion.pointers[i].coords}); + pointerMap.insert(Pointer{message.body.motion.pointers[i].properties, + message.body.motion.pointers[i].coords}); } - return Sample{nanoseconds{message.body.motion.eventTime}, pointers}; + return Sample{nanoseconds{message.body.motion.eventTime}, pointerMap}; } bool LegacyResampler::pointerPropertiesResampleable(const Sample& target, const Sample& auxiliary) { - if (target.pointers.size() > auxiliary.pointers.size()) { - LOG_IF(INFO, debugResampling()) - << "Not resampled. Auxiliary sample has fewer pointers than target sample."; - return false; - } - for (size_t i = 0; i < target.pointers.size(); ++i) { - if (target.pointers[i].properties.id != auxiliary.pointers[i].properties.id) { - LOG_IF(INFO, debugResampling()) << "Not resampled. Pointer ID mismatch."; + for (const Pointer& pointer : target.pointerMap) { + const std::optional auxiliaryPointer = + auxiliary.pointerMap.find(PointerMap::PointerId{pointer.properties.id}); + if (!auxiliaryPointer.has_value()) { + LOG_IF(INFO, debugResampling()) + << "Not resampled. Auxiliary sample does not contain all pointers from target."; return false; } - if (target.pointers[i].properties.toolType != auxiliary.pointers[i].properties.toolType) { + if (pointer.properties.toolType != auxiliaryPointer->properties.toolType) { LOG_IF(INFO, debugResampling()) << "Not resampled. Pointer ToolType mismatch."; return false; } - if (!canResampleTool(target.pointers[i].properties.toolType)) { + if (!canResampleTool(pointer.properties.toolType)) { LOG_IF(INFO, debugResampling()) << "Not resampled. Cannot resample " - << ftl::enum_string(target.pointers[i].properties.toolType) << " ToolType."; + << ftl::enum_string(pointer.properties.toolType) << " ToolType."; return false; } } @@ -173,28 +171,31 @@ bool LegacyResampler::canInterpolate(const InputMessage& message) const { } std::optional LegacyResampler::attemptInterpolation( - nanoseconds resampleTime, const InputMessage& futureSample) const { - if (!canInterpolate(futureSample)) { + nanoseconds resampleTime, const InputMessage& futureMessage) const { + if (!canInterpolate(futureMessage)) { return std::nullopt; } LOG_IF(FATAL, mLatestSamples.empty()) << "Not resampled. mLatestSamples must not be empty to interpolate."; const Sample& pastSample = *(mLatestSamples.end() - 1); + const Sample& futureSample = messageToSample(futureMessage); - const nanoseconds delta = - nanoseconds{futureSample.body.motion.eventTime} - pastSample.eventTime; + const nanoseconds delta = nanoseconds{futureSample.eventTime} - pastSample.eventTime; const float alpha = std::chrono::duration(resampleTime - pastSample.eventTime) / delta; - std::vector resampledPointers; - for (size_t i = 0; i < pastSample.pointers.size(); ++i) { - const PointerCoords& resampledCoords = - calculateResampledCoords(pastSample.pointers[i].coords, - futureSample.body.motion.pointers[i].coords, alpha); - resampledPointers.push_back(Pointer{pastSample.pointers[i].properties, resampledCoords}); + PointerMap resampledPointerMap; + for (const Pointer& pointer : pastSample.pointerMap) { + if (std::optional futureSamplePointer = + futureSample.pointerMap.find(PointerMap::PointerId{pointer.properties.id}); + futureSamplePointer.has_value()) { + const PointerCoords& resampledCoords = + calculateResampledCoords(pointer.coords, futureSamplePointer->coords, alpha); + resampledPointerMap.insert(Pointer{pointer.properties, resampledCoords}); + } } - return Sample{resampleTime, resampledPointers}; + return Sample{resampleTime, resampledPointerMap}; } bool LegacyResampler::canExtrapolate() const { @@ -247,14 +248,17 @@ std::optional LegacyResampler::attemptExtrapolation( std::chrono::duration(newResampleTime - pastSample.eventTime) / delta; - std::vector resampledPointers; - for (size_t i = 0; i < presentSample.pointers.size(); ++i) { - const PointerCoords& resampledCoords = - calculateResampledCoords(pastSample.pointers[i].coords, - presentSample.pointers[i].coords, alpha); - resampledPointers.push_back(Pointer{presentSample.pointers[i].properties, resampledCoords}); + PointerMap resampledPointerMap; + for (const Pointer& pointer : presentSample.pointerMap) { + if (std::optional pastSamplePointer = + pastSample.pointerMap.find(PointerMap::PointerId{pointer.properties.id}); + pastSamplePointer.has_value()) { + const PointerCoords& resampledCoords = + calculateResampledCoords(pastSamplePointer->coords, pointer.coords, alpha); + resampledPointerMap.insert(Pointer{pointer.properties, resampledCoords}); + } } - return Sample{newResampleTime, resampledPointers}; + return Sample{newResampleTime, resampledPointerMap}; } inline void LegacyResampler::addSampleToMotionEvent(const Sample& sample, @@ -267,6 +271,12 @@ nanoseconds LegacyResampler::getResampleLatency() const { return RESAMPLE_LATENCY; } +/** + * The resampler is unaware of ACTION_DOWN. Thus, it needs to constantly check for pointer IDs + * occurrences. This problem could be fixed if the resampler has access to the entire stream of + * MotionEvent actions. That way, both ACTION_DOWN and ACTION_UP will be visible; therefore, + * facilitating pointer tracking between samples. + */ void LegacyResampler::overwriteMotionEventSamples(MotionEvent& motionEvent) const { const size_t numSamples = motionEvent.getHistorySize() + 1; for (size_t sampleIndex = 0; sampleIndex < numSamples; ++sampleIndex) { @@ -276,22 +286,35 @@ void LegacyResampler::overwriteMotionEventSamples(MotionEvent& motionEvent) cons } void LegacyResampler::overwriteStillPointers(MotionEvent& motionEvent, size_t sampleIndex) const { + if (!mLastRealSample.has_value() || !mPreviousPrediction.has_value()) { + LOG_IF(INFO, debugResampling()) << "Still pointers not overwritten. Not enough data."; + return; + } for (size_t pointerIndex = 0; pointerIndex < motionEvent.getPointerCount(); ++pointerIndex) { + const std::optional lastRealPointer = mLastRealSample->pointerMap.find( + PointerMap::PointerId{motionEvent.getPointerId(pointerIndex)}); + const std::optional previousPointer = mPreviousPrediction->pointerMap.find( + PointerMap::PointerId{motionEvent.getPointerId(pointerIndex)}); + // This could happen because resampler only receives ACTION_MOVE events. + if (!lastRealPointer.has_value() || !previousPointer.has_value()) { + continue; + } const PointerCoords& pointerCoords = *(motionEvent.getHistoricalRawPointerCoords(pointerIndex, sampleIndex)); - if (equalXY(mLastRealSample->pointers[pointerIndex].coords, pointerCoords)) { + if (equalXY(pointerCoords, lastRealPointer->coords)) { LOG_IF(INFO, debugResampling()) << "Pointer ID: " << motionEvent.getPointerId(pointerIndex) << " did not move. Overwriting its coordinates from " << pointerCoords << " to " - << mLastRealSample->pointers[pointerIndex].coords; + << previousPointer->coords; setMotionEventPointerCoords(motionEvent, sampleIndex, pointerIndex, - mPreviousPrediction->pointers[pointerIndex].coords); + previousPointer->coords); } } } void LegacyResampler::overwriteOldPointers(MotionEvent& motionEvent, size_t sampleIndex) const { if (!mPreviousPrediction.has_value()) { + LOG_IF(INFO, debugResampling()) << "Old sample not overwritten. Not enough data."; return; } if (nanoseconds{motionEvent.getHistoricalEventTime(sampleIndex)} < @@ -302,8 +325,14 @@ void LegacyResampler::overwriteOldPointers(MotionEvent& motionEvent, size_t samp << mPreviousPrediction->eventTime.count() << "ns."; for (size_t pointerIndex = 0; pointerIndex < motionEvent.getPointerCount(); ++pointerIndex) { + const std::optional previousPointer = mPreviousPrediction->pointerMap.find( + PointerMap::PointerId{motionEvent.getPointerId(pointerIndex)}); + // This could happen because resampler only receives ACTION_MOVE events. + if (!previousPointer.has_value()) { + continue; + } setMotionEventPointerCoords(motionEvent, sampleIndex, pointerIndex, - mPreviousPrediction->pointers[pointerIndex].coords); + previousPointer->coords); } } } @@ -333,6 +362,7 @@ void LegacyResampler::resampleMotionEvent(nanoseconds frameTime, MotionEvent& mo mPreviousPrediction = sample; } } + LOG_IF(FATAL, mLatestSamples.empty()) << "mLatestSamples must contain at least one sample."; mLastRealSample = *(mLatestSamples.end() - 1); } diff --git a/libs/input/tests/Resampler_test.cpp b/libs/input/tests/Resampler_test.cpp index fae8518e87..3162b77c85 100644 --- a/libs/input/tests/Resampler_test.cpp +++ b/libs/input/tests/Resampler_test.cpp @@ -648,7 +648,15 @@ TEST_F(ResamplerTest, MultiplePointerDifferentIdOrderInterpolation) { mResampler->resampleMotionEvent(16ms, motionEvent, &futureSample); - assertMotionEventIsNotResampled(originalMotionEvent, motionEvent); + assertMotionEventIsResampledAndCoordsNear(originalMotionEvent, motionEvent, + {Pointer{.id = 0, + .x = 1.4f, + .y = 1.4f, + .isResampled = true}, + Pointer{.id = 1, + .x = 2.4f, + .y = 2.4f, + .isResampled = true}}); } TEST_F(ResamplerTest, MultiplePointerDifferentIdOrderExtrapolation) { @@ -670,7 +678,15 @@ TEST_F(ResamplerTest, MultiplePointerDifferentIdOrderExtrapolation) { mResampler->resampleMotionEvent(16ms, secondMotionEvent, /*futureSample=*/nullptr); - assertMotionEventIsNotResampled(secondOriginalMotionEvent, secondMotionEvent); + assertMotionEventIsResampledAndCoordsNear(secondOriginalMotionEvent, secondMotionEvent, + {Pointer{.id = 1, + .x = 4.4f, + .y = 4.4f, + .isResampled = true}, + Pointer{.id = 0, + .x = 3.4f, + .y = 3.4f, + .isResampled = true}}); } TEST_F(ResamplerTest, MultiplePointerDifferentIdsInterpolation) { -- cgit v1.2.3-59-g8ed1b From 37242c82244e56acf73458b83706d6b11e8b1289 Mon Sep 17 00:00:00 2001 From: Paul Ramirez Date: Fri, 18 Oct 2024 23:16:43 +0000 Subject: Do not resample when frameTime is not available LegacyResampler still resamples MotionEvents even when requestedFrameTime is std::nullopt. The problem is that if the parameter requestedFrameTime is std::nullopt, it is reassigned inside consumeBatchedInputEvents to std::numeric_limits::max(). In spite of using max() as a placeholder to indicate that everything should be consumed, it is still a valid time, and resampler uses it to resample MotionEvent. To fix this issue, having a valid requestedFrameTime should also be a condition to resample MotionEvents. Bug: 374375203 Flag: EXEMPT refactor Test: TEST=libinput_tests; m $TEST && $ANDROID_HOST_OUT/nativetest64/$TEST/$TEST Change-Id: I6e53bdd0a35d359da8b50f10dd4aad9bddc8aa3f --- include/input/InputConsumerNoResampling.h | 15 ++++++------ libs/input/InputConsumerNoResampling.cpp | 21 +++++++++-------- libs/input/tests/InputConsumerResampling_test.cpp | 28 +++++++++++++++++++++++ libs/input/tests/InputConsumer_test.cpp | 3 ++- 4 files changed, 50 insertions(+), 17 deletions(-) (limited to 'include/input') diff --git a/include/input/InputConsumerNoResampling.h b/include/input/InputConsumerNoResampling.h index 228347d818..2e346bb7cd 100644 --- a/include/input/InputConsumerNoResampling.h +++ b/include/input/InputConsumerNoResampling.h @@ -211,16 +211,17 @@ private: * `consumeBatchedInputEvents`. */ std::map> mBatches; + /** - * Creates a MotionEvent by consuming samples from the provided queue. If one message has - * eventTime > adjustedFrameTime, all subsequent messages in the queue will be skipped. It is - * assumed that messages are queued in chronological order. In other words, only events that - * occurred prior to the adjustedFrameTime will be consumed. - * @param requestedFrameTime the time up to which to consume events. - * @param messages the queue of messages to consume from + * Creates a MotionEvent by consuming samples from the provided queue. Consumes all messages + * with eventTime <= requestedFrameTime - resampleLatency, where `resampleLatency` is latency + * introduced by the resampler. Assumes that messages are queued in chronological order. + * @param requestedFrameTime The time up to which consume messages, as given by the inequality + * above. If std::nullopt, everything in messages will be consumed. + * @param messages the queue of messages to consume from. */ std::pair, std::optional> createBatchedMotionEvent( - const nsecs_t requestedFrameTime, std::queue& messages); + const std::optional requestedFrameTime, std::queue& messages); /** * Consumes the batched input events, optionally allowing the caller to specify a device id diff --git a/libs/input/InputConsumerNoResampling.cpp b/libs/input/InputConsumerNoResampling.cpp index 9665de799f..d3653cfc45 100644 --- a/libs/input/InputConsumerNoResampling.cpp +++ b/libs/input/InputConsumerNoResampling.cpp @@ -357,7 +357,8 @@ void InputConsumerNoResampling::handleMessages(std::vector&& messa mBatches[deviceId].emplace(msg); } else { // consume all pending batches for this device immediately - consumeBatchedInputEvents(deviceId, /*requestedFrameTime=*/std::nullopt); + consumeBatchedInputEvents(deviceId, /*requestedFrameTime=*/ + std::numeric_limits::max()); if (canResample && (action == AMOTION_EVENT_ACTION_UP || action == AMOTION_EVENT_ACTION_CANCEL)) { LOG_IF(INFO, mResamplers.erase(deviceId) == 0) @@ -480,7 +481,7 @@ void InputConsumerNoResampling::handleMessage(const InputMessage& msg) const { } std::pair, std::optional> -InputConsumerNoResampling::createBatchedMotionEvent(const nsecs_t requestedFrameTime, +InputConsumerNoResampling::createBatchedMotionEvent(const std::optional requestedFrameTime, std::queue& messages) { std::unique_ptr motionEvent; std::optional firstSeqForBatch; @@ -491,7 +492,11 @@ InputConsumerNoResampling::createBatchedMotionEvent(const nsecs_t requestedFrame const nanoseconds resampleLatency = (resampler != mResamplers.cend()) ? resampler->second->getResampleLatency() : nanoseconds{0}; - const nanoseconds adjustedFrameTime = nanoseconds{requestedFrameTime} - resampleLatency; + // When batching is not enabled, we want to consume all events. That's equivalent to having an + // infinite requestedFrameTime. + const nanoseconds adjustedFrameTime = (requestedFrameTime.has_value()) + ? (nanoseconds{*requestedFrameTime} - resampleLatency) + : nanoseconds{std::numeric_limits::max()}; while (!messages.empty() && (messages.front().body.motion.eventTime <= adjustedFrameTime.count())) { @@ -513,8 +518,9 @@ InputConsumerNoResampling::createBatchedMotionEvent(const nsecs_t requestedFrame if (!messages.empty()) { futureSample = &messages.front(); } - if ((motionEvent != nullptr) && (resampler != mResamplers.cend())) { - resampler->second->resampleMotionEvent(nanoseconds{requestedFrameTime}, *motionEvent, + if ((motionEvent != nullptr) && (resampler != mResamplers.cend()) && + (requestedFrameTime.has_value())) { + resampler->second->resampleMotionEvent(nanoseconds{*requestedFrameTime}, *motionEvent, futureSample); } @@ -524,16 +530,13 @@ InputConsumerNoResampling::createBatchedMotionEvent(const nsecs_t requestedFrame bool InputConsumerNoResampling::consumeBatchedInputEvents( std::optional deviceId, std::optional requestedFrameTime) { ensureCalledOnLooperThread(__func__); - // When batching is not enabled, we want to consume all events. That's equivalent to having an - // infinite requestedFrameTime. - requestedFrameTime = requestedFrameTime.value_or(std::numeric_limits::max()); bool producedEvents = false; for (auto deviceIdIter = (deviceId.has_value()) ? (mBatches.find(*deviceId)) : (mBatches.begin()); deviceIdIter != mBatches.cend(); ++deviceIdIter) { std::queue& messages = deviceIdIter->second; - auto [motion, firstSeqForBatch] = createBatchedMotionEvent(*requestedFrameTime, messages); + auto [motion, firstSeqForBatch] = createBatchedMotionEvent(requestedFrameTime, messages); if (motion != nullptr) { LOG_ALWAYS_FATAL_IF(!firstSeqForBatch.has_value()); mCallbacks.onMotionEvent(std::move(motion), *firstSeqForBatch); diff --git a/libs/input/tests/InputConsumerResampling_test.cpp b/libs/input/tests/InputConsumerResampling_test.cpp index 883ca82fe0..a7feeeb301 100644 --- a/libs/input/tests/InputConsumerResampling_test.cpp +++ b/libs/input/tests/InputConsumerResampling_test.cpp @@ -566,4 +566,32 @@ TEST_F(InputConsumerResamplingTest, OldEventReceivedAfterResampleOccurs) { mClientTestChannel->assertFinishMessage(/*seq=*/4, /*handled=*/true); } +TEST_F(InputConsumerResamplingTest, DoNotResampleWhenFrameTimeIsNotAvailable) { + 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}}); + + 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(std::nullopt); + 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}}); + + mClientTestChannel->assertFinishMessage(/*seq=*/1, /*handled=*/true); + mClientTestChannel->assertFinishMessage(/*seq=*/2, /*handled=*/true); + mClientTestChannel->assertFinishMessage(/*seq=*/3, /*handled=*/true); +} + } // namespace android diff --git a/libs/input/tests/InputConsumer_test.cpp b/libs/input/tests/InputConsumer_test.cpp index 6a3bbe5b36..06e19bb644 100644 --- a/libs/input/tests/InputConsumer_test.cpp +++ b/libs/input/tests/InputConsumer_test.cpp @@ -194,7 +194,7 @@ TEST_F(InputConsumerTest, MessageStreamBatchedInMotionEvent) { std::unique_ptr moveMotionEvent = assertReceivedMotionEvent(WithMotionAction(ACTION_MOVE)); ASSERT_NE(moveMotionEvent, nullptr); - EXPECT_EQ(moveMotionEvent->getHistorySize() + 1, 3UL); + EXPECT_EQ(moveMotionEvent->getHistorySize() + 1, 2UL); mClientTestChannel->assertFinishMessage(/*seq=*/0, /*handled=*/true); mClientTestChannel->assertFinishMessage(/*seq=*/1, /*handled=*/true); @@ -443,4 +443,5 @@ TEST_F(InputConsumerTest, MultiDeviceResampling) { mClientTestChannel->assertFinishMessage(/*seq=*/8, /*handled=*/true); mClientTestChannel->assertFinishMessage(/*seq=*/10, /*handled=*/true); } + } // namespace android -- cgit v1.2.3-59-g8ed1b From 08ee19997d0ad4fab38465ef878b666c9fffb203 Mon Sep 17 00:00:00 2001 From: Paul Ramirez Date: Thu, 10 Oct 2024 18:02:15 +0000 Subject: Reimplement Chromium's OneEuroFilter to InputConsumer Reimplemented Chromium's OneEuroFilter usage to InputConsumerNoResampling. There are a few differences between Chromium's work and this CL. We reimplemented One Euro filter an adaptive cutoff frequency low pass made in this implementation as in the Chromium's implementation. The class FilteredResampler filters the output of LegacyResampler using the One Euro filter approach. Here's the link to Chromium's to One Euro filter: https://source.chromium.org/chromium/chromium/src/+/main:ui/base/prediction/one_euro_filter.h Bug: 297226446 Flag: EXEMPT bugfix Test: TEST=libinput_tests; m $TEST && $ANDROID_HOST_OUT/nativetest64/$TEST/$TEST Change-Id: I0316cb1e81c73b1dc28dc809f55dee3a1cc0ebd2 --- include/input/CoordinateFilter.h | 54 +++++++++++++ include/input/OneEuroFilter.h | 101 ++++++++++++++++++++++++ include/input/Resampler.h | 41 ++++++++++ libs/input/Android.bp | 2 + libs/input/CoordinateFilter.cpp | 31 ++++++++ libs/input/OneEuroFilter.cpp | 79 +++++++++++++++++++ libs/input/Resampler.cpp | 30 +++++++ libs/input/tests/Android.bp | 1 + libs/input/tests/OneEuroFilter_test.cpp | 134 ++++++++++++++++++++++++++++++++ 9 files changed, 473 insertions(+) create mode 100644 include/input/CoordinateFilter.h create mode 100644 include/input/OneEuroFilter.h create mode 100644 libs/input/CoordinateFilter.cpp create mode 100644 libs/input/OneEuroFilter.cpp create mode 100644 libs/input/tests/OneEuroFilter_test.cpp (limited to 'include/input') diff --git a/include/input/CoordinateFilter.h b/include/input/CoordinateFilter.h new file mode 100644 index 0000000000..f36472dc8c --- /dev/null +++ b/include/input/CoordinateFilter.h @@ -0,0 +1,54 @@ +/** + * Copyright 2024 The Android Open Source Project + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +#pragma once + +#include + +#include +#include + +namespace android { + +/** + * Pair of OneEuroFilters that independently filter X and Y coordinates. Both filters share the same + * constructor's parameters. The minimum cutoff frequency is the base cutoff frequency, that is, the + * resulting cutoff frequency in the absence of signal's speed. Likewise, beta is a scaling factor + * of the signal's speed that sets how much the signal's speed contributes to the resulting cutoff + * frequency. The adaptive cutoff frequency criterion is f_c = f_c_min + β|̇x_filtered| + */ +class CoordinateFilter { +public: + explicit CoordinateFilter(float minCutoffFreq, float beta); + + /** + * Filters in place only the AXIS_X and AXIS_Y fields from coords. Each call to filter must + * provide a timestamp strictly greater than the timestamp of the previous call. The first time + * this method is invoked no filtering takes place. Subsequent calls do overwrite `coords` with + * filtered data. + * + * @param timestamp The timestamps at which to filter. It must be greater than the one passed in + * the previous call. + * @param coords Coordinates to be overwritten by the corresponding filtered coordinates. + */ + void filter(std::chrono::duration timestamp, PointerCoords& coords); + +private: + OneEuroFilter mXFilter; + OneEuroFilter mYFilter; +}; + +} // namespace android \ No newline at end of file diff --git a/include/input/OneEuroFilter.h b/include/input/OneEuroFilter.h new file mode 100644 index 0000000000..a0168e4f91 --- /dev/null +++ b/include/input/OneEuroFilter.h @@ -0,0 +1,101 @@ +/** + * Copyright 2024 The Android Open Source Project + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +#pragma once + +#include +#include + +#include + +namespace android { + +/** + * Low pass filter with adaptive low pass frequency based on the signal's speed. The signal's cutoff + * frequency is determined by f_c = f_c_min + β|̇x_filtered|. Refer to + * https://dl.acm.org/doi/10.1145/2207676.2208639 for details on how the filter works and how to + * tune it. + */ +class OneEuroFilter { +public: + /** + * Default cutoff frequency of the filtered signal's speed. 1.0 Hz is the value in the filter's + * paper. + */ + static constexpr float kDefaultSpeedCutoffFreq = 1.0; + + OneEuroFilter() = delete; + + explicit OneEuroFilter(float minCutoffFreq, float beta, + float speedCutoffFreq = kDefaultSpeedCutoffFreq); + + OneEuroFilter(const OneEuroFilter&) = delete; + OneEuroFilter& operator=(const OneEuroFilter&) = delete; + OneEuroFilter(OneEuroFilter&&) = delete; + OneEuroFilter& operator=(OneEuroFilter&&) = delete; + + /** + * Returns the filtered value of rawPosition. Each call to filter must provide a timestamp + * strictly greater than the timestamp of the previous call. The first time the method is + * called, it returns the value of rawPosition. Any subsequent calls provide a filtered value. + * + * @param timestamp The timestamp at which to filter. It must be strictly greater than the one + * provided in the previous call. + * @param rawPosition Position to be filtered. + */ + float filter(std::chrono::duration timestamp, float rawPosition); + +private: + /** + * Minimum cutoff frequency. This is the constant term in the adaptive cutoff frequency + * criterion. Units are Hertz. + */ + const float mMinCutoffFreq; + + /** + * Slope of the cutoff frequency criterion. This is the term scaling the absolute value of the + * filtered signal's speed. The data member is dimensionless, that is, it does not have units. + */ + const float mBeta; + + /** + * Cutoff frequency of the signal's speed. This is the cutoff frequency applied to the filtering + * of the signal's speed. Units are Hertz. + */ + const float mSpeedCutoffFreq; + + /** + * The timestamp from the previous call. Units are seconds. + */ + std::optional> mPrevTimestamp; + + /** + * The raw position from the previous call. + */ + std::optional mPrevRawPosition; + + /** + * The filtered velocity from the previous call. Units are position per second. + */ + std::optional mPrevFilteredVelocity; + + /** + * The filtered position from the previous call. + */ + std::optional mPrevFilteredPosition; +}; + +} // namespace android diff --git a/include/input/Resampler.h b/include/input/Resampler.h index 6d95ca7e86..155097732c 100644 --- a/include/input/Resampler.h +++ b/include/input/Resampler.h @@ -19,11 +19,13 @@ #include #include #include +#include #include #include #include #include +#include #include #include #include @@ -293,4 +295,43 @@ private: inline static void addSampleToMotionEvent(const Sample& sample, MotionEvent& motionEvent); }; +/** + * Resampler that first applies the LegacyResampler resampling algorithm, then independently filters + * the X and Y coordinates with a pair of One Euro filters. + */ +class FilteredLegacyResampler final : public Resampler { +public: + /** + * Creates a resampler, using the given minCutoffFreq and beta to instantiate its One Euro + * filters. + */ + explicit FilteredLegacyResampler(float minCutoffFreq, float beta); + + void resampleMotionEvent(std::chrono::nanoseconds requestedFrameTime, MotionEvent& motionEvent, + const InputMessage* futureMessage) override; + + std::chrono::nanoseconds getResampleLatency() const override; + +private: + LegacyResampler mResampler; + + /** + * Minimum cutoff frequency of the value's low pass filter. Refer to OneEuroFilter class for a + * more detailed explanation. + */ + const float mMinCutoffFreq; + + /** + * Scaling factor of the adaptive cutoff frequency criterion. Refer to OneEuroFilter class for a + * more detailed explanation. + */ + const float mBeta; + + /* + * Note: an associative array with constant insertion and lookup times would be more efficient. + * When this was implemented, there was no container with these properties. + */ + std::map mFilteredPointers; +}; + } // namespace android diff --git a/libs/input/Android.bp b/libs/input/Android.bp index e4e81adf58..a4ae54b351 100644 --- a/libs/input/Android.bp +++ b/libs/input/Android.bp @@ -217,6 +217,7 @@ cc_library { ], srcs: [ "AccelerationCurve.cpp", + "CoordinateFilter.cpp", "Input.cpp", "InputConsumer.cpp", "InputConsumerNoResampling.cpp", @@ -230,6 +231,7 @@ cc_library { "KeyLayoutMap.cpp", "MotionPredictor.cpp", "MotionPredictorMetricsManager.cpp", + "OneEuroFilter.cpp", "PrintTools.cpp", "PropertyMap.cpp", "Resampler.cpp", diff --git a/libs/input/CoordinateFilter.cpp b/libs/input/CoordinateFilter.cpp new file mode 100644 index 0000000000..d231474577 --- /dev/null +++ b/libs/input/CoordinateFilter.cpp @@ -0,0 +1,31 @@ +/** + * Copyright 2024 The Android Open Source Project + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +#define LOG_TAG "CoordinateFilter" + +#include + +namespace android { + +CoordinateFilter::CoordinateFilter(float minCutoffFreq, float beta) + : mXFilter{minCutoffFreq, beta}, mYFilter{minCutoffFreq, beta} {} + +void CoordinateFilter::filter(std::chrono::duration timestamp, PointerCoords& coords) { + coords.setAxisValue(AMOTION_EVENT_AXIS_X, mXFilter.filter(timestamp, coords.getX())); + coords.setAxisValue(AMOTION_EVENT_AXIS_Y, mYFilter.filter(timestamp, coords.getY())); +} + +} // namespace android diff --git a/libs/input/OneEuroFilter.cpp b/libs/input/OneEuroFilter.cpp new file mode 100644 index 0000000000..400d7c9ab0 --- /dev/null +++ b/libs/input/OneEuroFilter.cpp @@ -0,0 +1,79 @@ +/** + * Copyright 2024 The Android Open Source Project + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +#define LOG_TAG "OneEuroFilter" + +#include +#include + +#include +#include + +namespace android { +namespace { + +inline float cutoffFreq(float minCutoffFreq, float beta, float filteredSpeed) { + return minCutoffFreq + beta * std::abs(filteredSpeed); +} + +inline float smoothingFactor(std::chrono::duration samplingPeriod, float cutoffFreq) { + return samplingPeriod.count() / (samplingPeriod.count() + (1.0 / (2.0 * M_PI * cutoffFreq))); +} + +inline float lowPassFilter(float rawPosition, float prevFilteredPosition, float smoothingFactor) { + return smoothingFactor * rawPosition + (1 - smoothingFactor) * prevFilteredPosition; +} + +} // namespace + +OneEuroFilter::OneEuroFilter(float minCutoffFreq, float beta, float speedCutoffFreq) + : mMinCutoffFreq{minCutoffFreq}, mBeta{beta}, mSpeedCutoffFreq{speedCutoffFreq} {} + +float OneEuroFilter::filter(std::chrono::duration timestamp, float rawPosition) { + LOG_IF(FATAL, mPrevFilteredPosition.has_value() && (timestamp <= *mPrevTimestamp)) + << "Timestamp must be greater than mPrevTimestamp"; + + const std::chrono::duration samplingPeriod = (mPrevTimestamp.has_value()) + ? (timestamp - *mPrevTimestamp) + : std::chrono::duration{1.0}; + + const float rawVelocity = (mPrevFilteredPosition.has_value()) + ? ((rawPosition - *mPrevFilteredPosition) / samplingPeriod.count()) + : 0.0; + + const float speedSmoothingFactor = smoothingFactor(samplingPeriod, mSpeedCutoffFreq); + + const float filteredVelocity = (mPrevFilteredVelocity.has_value()) + ? lowPassFilter(rawVelocity, *mPrevFilteredVelocity, speedSmoothingFactor) + : rawVelocity; + + const float positionCutoffFreq = cutoffFreq(mMinCutoffFreq, mBeta, filteredVelocity); + + const float positionSmoothingFactor = smoothingFactor(samplingPeriod, positionCutoffFreq); + + const float filteredPosition = (mPrevFilteredPosition.has_value()) + ? lowPassFilter(rawPosition, *mPrevFilteredPosition, positionSmoothingFactor) + : rawPosition; + + mPrevTimestamp = timestamp; + mPrevRawPosition = rawPosition; + mPrevFilteredVelocity = filteredVelocity; + mPrevFilteredPosition = filteredPosition; + + return filteredPosition; +} + +} // namespace android diff --git a/libs/input/Resampler.cpp b/libs/input/Resampler.cpp index 056db093d1..3ab132d550 100644 --- a/libs/input/Resampler.cpp +++ b/libs/input/Resampler.cpp @@ -389,4 +389,34 @@ void LegacyResampler::resampleMotionEvent(nanoseconds frameTime, MotionEvent& mo mLastRealSample = *(mLatestSamples.end() - 1); } +// --- FilteredLegacyResampler --- + +FilteredLegacyResampler::FilteredLegacyResampler(float minCutoffFreq, float beta) + : mResampler{}, mMinCutoffFreq{minCutoffFreq}, mBeta{beta} {} + +void FilteredLegacyResampler::resampleMotionEvent(std::chrono::nanoseconds requestedFrameTime, + MotionEvent& motionEvent, + const InputMessage* futureSample) { + mResampler.resampleMotionEvent(requestedFrameTime, motionEvent, futureSample); + const size_t numSamples = motionEvent.getHistorySize() + 1; + for (size_t sampleIndex = 0; sampleIndex < numSamples; ++sampleIndex) { + for (size_t pointerIndex = 0; pointerIndex < motionEvent.getPointerCount(); + ++pointerIndex) { + const int32_t pointerId = motionEvent.getPointerProperties(pointerIndex)->id; + const nanoseconds eventTime = + nanoseconds{motionEvent.getHistoricalEventTime(sampleIndex)}; + // Refer to the static function `setMotionEventPointerCoords` for a justification of + // casting away const. + PointerCoords& pointerCoords = const_cast( + *(motionEvent.getHistoricalRawPointerCoords(pointerIndex, sampleIndex))); + const auto& [iter, _] = mFilteredPointers.try_emplace(pointerId, mMinCutoffFreq, mBeta); + iter->second.filter(eventTime, pointerCoords); + } + } +} + +std::chrono::nanoseconds FilteredLegacyResampler::getResampleLatency() const { + return mResampler.getResampleLatency(); +} + } // namespace android diff --git a/libs/input/tests/Android.bp b/libs/input/tests/Android.bp index 661c9f739f..46e819061f 100644 --- a/libs/input/tests/Android.bp +++ b/libs/input/tests/Android.bp @@ -25,6 +25,7 @@ cc_test { "InputVerifier_test.cpp", "MotionPredictor_test.cpp", "MotionPredictorMetricsManager_test.cpp", + "OneEuroFilter_test.cpp", "Resampler_test.cpp", "RingBuffer_test.cpp", "TestInputChannel.cpp", diff --git a/libs/input/tests/OneEuroFilter_test.cpp b/libs/input/tests/OneEuroFilter_test.cpp new file mode 100644 index 0000000000..270e789c84 --- /dev/null +++ b/libs/input/tests/OneEuroFilter_test.cpp @@ -0,0 +1,134 @@ +/** + * Copyright 2024 The Android Open Source Project + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +#include + +#include +#include +#include +#include +#include + +#include + +#include + +namespace android { +namespace { + +using namespace std::literals::chrono_literals; +using std::chrono::duration; + +struct Sample { + duration timestamp{}; + double value{}; + + friend bool operator<(const Sample& lhs, const Sample& rhs) { return lhs.value < rhs.value; } +}; + +/** + * Generates a sinusoidal signal with the passed frequency and amplitude. + */ +std::vector generateSinusoidalSignal(duration signalDuration, + double samplingFrequency, double signalFrequency, + double amplitude) { + std::vector signal; + const duration samplingPeriod{1.0 / samplingFrequency}; + for (duration timestamp{0.0}; timestamp < signalDuration; timestamp += samplingPeriod) { + signal.push_back( + Sample{timestamp, + amplitude * std::sin(2.0 * M_PI * signalFrequency * timestamp.count())}); + } + return signal; +} + +double meanAbsoluteError(const std::vector& filteredSignal, + const std::vector& signal) { + if (filteredSignal.size() != signal.size()) { + ADD_FAILURE() << "filteredSignal and signal do not have equal number of samples"; + return std::numeric_limits::max(); + } + std::vector absoluteError; + for (size_t sampleIndex = 0; sampleIndex < signal.size(); ++sampleIndex) { + absoluteError.push_back( + std::abs(filteredSignal[sampleIndex].value - signal[sampleIndex].value)); + } + if (absoluteError.empty()) { + ADD_FAILURE() << "Zero division. absoluteError is empty"; + return std::numeric_limits::max(); + } + return std::accumulate(absoluteError.begin(), absoluteError.end(), 0.0) / absoluteError.size(); +} + +double maxAbsoluteAmplitude(const std::vector& signal) { + if (signal.empty()) { + ADD_FAILURE() << "Max absolute value amplitude does not exist. Signal is empty"; + return std::numeric_limits::max(); + } + std::vector absoluteSignal; + for (const Sample& sample : signal) { + absoluteSignal.push_back(Sample{sample.timestamp, std::abs(sample.value)}); + } + return std::max_element(absoluteSignal.begin(), absoluteSignal.end())->value; +} + +} // namespace + +class OneEuroFilterTest : public ::testing::Test { +protected: + // The constructor's parameters are the ones that Chromium's using. The tuning was based on a 60 + // Hz sampling frequency. Refer to their one_euro_filter.h header for additional information + // about these parameters. + OneEuroFilterTest() : mFilter{/*minCutoffFreq=*/4.7, /*beta=*/0.01} {} + + std::vector filterSignal(const std::vector& signal) { + std::vector filteredSignal; + for (const Sample& sample : signal) { + filteredSignal.push_back( + Sample{sample.timestamp, mFilter.filter(sample.timestamp, sample.value)}); + } + return filteredSignal; + } + + OneEuroFilter mFilter; +}; + +TEST_F(OneEuroFilterTest, PassLowFrequencySignal) { + const std::vector signal = + generateSinusoidalSignal(1s, /*samplingFrequency=*/60, /*signalFrequency=*/1, + /*amplitude=*/1); + + const std::vector filteredSignal = filterSignal(signal); + + // The reason behind using the mean absolute error as a metric is that, ideally, a low frequency + // filtered signal is expected to be almost identical to the raw one. Therefore, the error + // between them should be minimal. The constant is heuristically chosen. + EXPECT_LT(meanAbsoluteError(filteredSignal, signal), 0.25); +} + +TEST_F(OneEuroFilterTest, RejectHighFrequencySignal) { + const std::vector signal = + generateSinusoidalSignal(1s, /*samplingFrequency=*/60, /*signalFrequency=*/22.5, + /*amplitude=*/1); + + const std::vector filteredSignal = filterSignal(signal); + + // The filtered signal should consist of values that are much closer to zero. The comparison + // constant is heuristically chosen. + EXPECT_LT(maxAbsoluteAmplitude(filteredSignal), 0.25); +} + +} // namespace android -- cgit v1.2.3-59-g8ed1b From ffe24ccf66d9179847323e6dfa301b75ee06f2b4 Mon Sep 17 00:00:00 2001 From: DingYong Date: Thu, 15 Aug 2024 08:24:02 +0000 Subject: Add keyboard volume mute LED. (2/2) (cherry picked from https://partner-android-review.googlesource.com/q/commit:80f60ec95afc0c5995647aa19f87b5389f656c25) Change-Id: I75f68a34b2557deeb9e5d760480e5ab4aa6af328 Bug: 373556678 Flag: NONE external OEM contribution only active in their devices --- include/input/InputDevice.h | 3 ++- services/inputflinger/reader/EventHub.cpp | 3 ++- services/inputflinger/reader/controller/PeripheralController.cpp | 2 ++ services/inputflinger/reader/include/EventHub.h | 2 ++ 4 files changed, 8 insertions(+), 2 deletions(-) (limited to 'include/input') diff --git a/include/input/InputDevice.h b/include/input/InputDevice.h index 6a248ef188..6b45dd39dc 100644 --- a/include/input/InputDevice.h +++ b/include/input/InputDevice.h @@ -131,8 +131,9 @@ enum class InputDeviceLightType : int32_t { PLAYER_ID = 1, KEYBOARD_BACKLIGHT = 2, KEYBOARD_MIC_MUTE = 3, + KEYBOARD_VOLUME_MUTE = 4, - ftl_last = KEYBOARD_MIC_MUTE + ftl_last = KEYBOARD_VOLUME_MUTE }; enum class InputDeviceLightCapability : uint32_t { diff --git a/services/inputflinger/reader/EventHub.cpp b/services/inputflinger/reader/EventHub.cpp index 35ba48f137..013ef862ad 100644 --- a/services/inputflinger/reader/EventHub.cpp +++ b/services/inputflinger/reader/EventHub.cpp @@ -129,7 +129,8 @@ static const std::unordered_map LIGHT_CLASSES = {"multi_intensity", InputLightClass::MULTI_INTENSITY}, {"max_brightness", InputLightClass::MAX_BRIGHTNESS}, {"kbd_backlight", InputLightClass::KEYBOARD_BACKLIGHT}, - {"mic_mute", InputLightClass::KEYBOARD_MIC_MUTE}}; + {"mic_mute", InputLightClass::KEYBOARD_MIC_MUTE}, + {"mute", InputLightClass::KEYBOARD_VOLUME_MUTE}}; // Mapping for input multicolor led class node names. // https://www.kernel.org/doc/html/latest/leds/leds-class-multicolor.html diff --git a/services/inputflinger/reader/controller/PeripheralController.cpp b/services/inputflinger/reader/controller/PeripheralController.cpp index 49ad8b5d69..9eeb2b2a2c 100644 --- a/services/inputflinger/reader/controller/PeripheralController.cpp +++ b/services/inputflinger/reader/controller/PeripheralController.cpp @@ -514,6 +514,8 @@ void PeripheralController::configureLights() { type = InputDeviceLightType::KEYBOARD_BACKLIGHT; } else if (rawInfo.flags.test(InputLightClass::KEYBOARD_MIC_MUTE)) { type = InputDeviceLightType::KEYBOARD_MIC_MUTE; + } else if (rawInfo.flags.test(InputLightClass::KEYBOARD_VOLUME_MUTE)) { + type = InputDeviceLightType::KEYBOARD_VOLUME_MUTE; } else { type = InputDeviceLightType::INPUT; } diff --git a/services/inputflinger/reader/include/EventHub.h b/services/inputflinger/reader/include/EventHub.h index 4336945e96..5839b4c41c 100644 --- a/services/inputflinger/reader/include/EventHub.h +++ b/services/inputflinger/reader/include/EventHub.h @@ -179,6 +179,8 @@ enum class InputLightClass : uint32_t { KEYBOARD_BACKLIGHT = 0x00000100, /* The input light has mic_mute name */ KEYBOARD_MIC_MUTE = 0x00000200, + /* The input light has mute name */ + KEYBOARD_VOLUME_MUTE = 0x00000400, }; enum class InputBatteryClass : uint32_t { -- cgit v1.2.3-59-g8ed1b From d62330d4811cc0277e8608ce7f3867cbf4d53564 Mon Sep 17 00:00:00 2001 From: Harry Cutts Date: Mon, 11 Nov 2024 13:43:13 +0000 Subject: KeyboardInputMapper: migrate most tests to InputMapperUnitTest Some of these tests create multiple input devices and mappers, which I didn't have time to work out how to mimic using InputMapperUnitTest. The rest have been migrated. Notes: * Split out Process_UnknownKey from Process_SimpleKeyPress, since it's a separate behaviour to test * As far as I can tell, Process_KeyRemapping was just testing the remapping code in FakeEventHub, so I removed it * Process_WhenNotOrientationAware_ShouldNotRotateDPad now checks for a display ID of DISPLAY_ID, not INVALID. This is because the method called by KeyboardInputMapper to check orientation (getAssociatedViewport) will never return an orientation without also returning a valid display ID; setting one without the other appears to have been possible in the old testing infrastructure, but would only have affected state somewhere outside of KeyboardInputMapper. For similar reasons, removed the second half of DisplayIdConfigurationChange_NotOrientationAware. * MarkSupportedKeyCodes was removed, as the code to test it would have been too bulky to test a trivial one-liner (and it would basically have been a change detector). Test: atest --host inputflinger_tests Bug: 283812079 Flag: TEST_ONLY Change-Id: Id6d2eb4ec2bae6bd0e9a263209169785e40a6024 --- include/input/Input.h | 2 + services/inputflinger/reader/include/InputDevice.h | 8 +- services/inputflinger/tests/InputMapperTest.cpp | 7 +- services/inputflinger/tests/InputMapperTest.h | 2 + services/inputflinger/tests/InterfaceMocks.h | 10 +- .../tests/KeyboardInputMapper_test.cpp | 1063 ++++++++++---------- 6 files changed, 544 insertions(+), 548 deletions(-) (limited to 'include/input') diff --git a/include/input/Input.h b/include/input/Input.h index a8684bd19b..127046da82 100644 --- a/include/input/Input.h +++ b/include/input/Input.h @@ -294,6 +294,8 @@ enum class KeyboardType { NONE = AINPUT_KEYBOARD_TYPE_NONE, NON_ALPHABETIC = AINPUT_KEYBOARD_TYPE_NON_ALPHABETIC, ALPHABETIC = AINPUT_KEYBOARD_TYPE_ALPHABETIC, + ftl_first = NONE, + ftl_last = ALPHABETIC, }; bool isStylusToolType(ToolType toolType); diff --git a/services/inputflinger/reader/include/InputDevice.h b/services/inputflinger/reader/include/InputDevice.h index abe7a5fa6e..4744dd0e4e 100644 --- a/services/inputflinger/reader/include/InputDevice.h +++ b/services/inputflinger/reader/include/InputDevice.h @@ -48,7 +48,7 @@ public: inline InputReaderContext* getContext() { return mContext; } inline int32_t getId() const { return mId; } inline int32_t getControllerNumber() const { return mControllerNumber; } - inline int32_t getGeneration() const { return mGeneration; } + inline virtual int32_t getGeneration() const { return mGeneration; } inline const std::string getName() const { return mIdentifier.name; } inline const std::string getDescriptor() { return mIdentifier.descriptor; } inline std::optional getBluetoothAddress() const { @@ -59,7 +59,7 @@ public: inline virtual uint32_t getSources() const { return mSources; } inline bool hasEventHubDevices() const { return !mDevices.empty(); } - inline bool isExternal() { return mIsExternal; } + inline virtual bool isExternal() { return mIsExternal; } inline std::optional getAssociatedDisplayPort() const { return mAssociatedDisplayPort; } @@ -79,7 +79,7 @@ public: inline bool isIgnored() { return !getMapperCount() && !mController; } - inline KeyboardType getKeyboardType() const { return mKeyboardType; } + inline virtual KeyboardType getKeyboardType() const { return mKeyboardType; } bool isEnabled(); @@ -124,7 +124,7 @@ public: int32_t getMetaState(); void setKeyboardType(KeyboardType keyboardType); - void bumpGeneration(); + virtual void bumpGeneration(); [[nodiscard]] NotifyDeviceResetArgs notifyReset(nsecs_t when); diff --git a/services/inputflinger/tests/InputMapperTest.cpp b/services/inputflinger/tests/InputMapperTest.cpp index ca797dce95..8235c90da6 100644 --- a/services/inputflinger/tests/InputMapperTest.cpp +++ b/services/inputflinger/tests/InputMapperTest.cpp @@ -100,9 +100,14 @@ std::list InputMapperUnitTest::process(int32_t type, int32_t code, i std::list InputMapperUnitTest::process(nsecs_t when, int32_t type, int32_t code, int32_t value) { + return process(when, when, type, code, value); +} + +std::list InputMapperUnitTest::process(nsecs_t when, nsecs_t readTime, int32_t type, + int32_t code, int32_t value) { RawEvent event; event.when = when; - event.readTime = when; + event.readTime = readTime; event.deviceId = mMapper->getDeviceContext().getEventHubId(); event.type = type; event.code = code; diff --git a/services/inputflinger/tests/InputMapperTest.h b/services/inputflinger/tests/InputMapperTest.h index b6c5812ec5..10ef6f1660 100644 --- a/services/inputflinger/tests/InputMapperTest.h +++ b/services/inputflinger/tests/InputMapperTest.h @@ -56,6 +56,8 @@ protected: std::list process(int32_t type, int32_t code, int32_t value); std::list process(nsecs_t when, int32_t type, int32_t code, int32_t value); + std::list process(nsecs_t when, nsecs_t readTime, int32_t type, int32_t code, + int32_t value); InputDeviceIdentifier mIdentifier; MockEventHubInterface mMockEventHub; diff --git a/services/inputflinger/tests/InterfaceMocks.h b/services/inputflinger/tests/InterfaceMocks.h index 25e2b4557e..6f7c2e5271 100644 --- a/services/inputflinger/tests/InterfaceMocks.h +++ b/services/inputflinger/tests/InterfaceMocks.h @@ -201,7 +201,9 @@ public: MOCK_METHOD(uint32_t, getSources, (), (const, override)); MOCK_METHOD(std::optional, getAssociatedViewport, (), (const)); + MOCK_METHOD(KeyboardType, getKeyboardType, (), (const, override)); MOCK_METHOD(bool, isEnabled, (), ()); + MOCK_METHOD(bool, isExternal, (), (override)); MOCK_METHOD(void, dump, (std::string& dump, const std::string& eventHubDevStr), ()); MOCK_METHOD(void, addEmptyEventHubDevice, (int32_t eventHubId), ()); @@ -249,8 +251,6 @@ public: MOCK_METHOD(int32_t, getMetaState, (), ()); MOCK_METHOD(void, setKeyboardType, (KeyboardType keyboardType), ()); - MOCK_METHOD(void, bumpGeneration, (), ()); - MOCK_METHOD(const PropertyMap&, getConfiguration, (), (const, override)); MOCK_METHOD(NotifyDeviceResetArgs, notifyReset, (nsecs_t when), ()); @@ -260,5 +260,11 @@ public: MOCK_METHOD(void, updateLedState, (bool reset), ()); MOCK_METHOD(size_t, getMapperCount, (), ()); + + virtual int32_t getGeneration() const override { return mGeneration; } + virtual void bumpGeneration() override { mGeneration++; } + +private: + int32_t mGeneration = 0; }; } // namespace android diff --git a/services/inputflinger/tests/KeyboardInputMapper_test.cpp b/services/inputflinger/tests/KeyboardInputMapper_test.cpp index 7474b2d119..39b583c6ae 100644 --- a/services/inputflinger/tests/KeyboardInputMapper_test.cpp +++ b/services/inputflinger/tests/KeyboardInputMapper_test.cpp @@ -33,6 +33,7 @@ #include #include #include +#include #include "EventHub.h" #include "InputMapperTest.h" @@ -45,11 +46,16 @@ namespace android { +using namespace ftl::flag_operators; using testing::_; using testing::AllOf; +using testing::AnyOf; using testing::Args; using testing::DoAll; +using testing::IsEmpty; using testing::Return; +using testing::ReturnArg; +using testing::SaveArg; using testing::SetArgPointee; using testing::VariantWith; @@ -59,7 +65,23 @@ namespace { constexpr ui::LogicalDisplayId DISPLAY_ID = ui::LogicalDisplayId::DEFAULT; constexpr int32_t DISPLAY_WIDTH = 480; constexpr int32_t DISPLAY_HEIGHT = 800; -constexpr std::optional NO_PORT = std::nullopt; // no physical port is specified + +DisplayViewport createPrimaryViewport(ui::Rotation orientation) { + const bool isRotated = + orientation == ui::Rotation::Rotation90 || orientation == ui::Rotation::Rotation270; + DisplayViewport v; + v.displayId = DISPLAY_ID; + v.orientation = orientation; + v.logicalRight = isRotated ? DISPLAY_HEIGHT : DISPLAY_WIDTH; + v.logicalBottom = isRotated ? DISPLAY_WIDTH : DISPLAY_HEIGHT; + v.physicalRight = isRotated ? DISPLAY_HEIGHT : DISPLAY_WIDTH; + v.physicalBottom = isRotated ? DISPLAY_WIDTH : DISPLAY_HEIGHT; + v.deviceWidth = isRotated ? DISPLAY_HEIGHT : DISPLAY_WIDTH; + v.deviceHeight = isRotated ? DISPLAY_WIDTH : DISPLAY_HEIGHT; + v.isActive = true; + v.uniqueId = "local:1"; + return v; +} } // namespace @@ -68,6 +90,8 @@ constexpr std::optional NO_PORT = std::nullopt; // no physical port is */ class KeyboardInputMapperUnitTest : public InputMapperUnitTest { protected: + const KeyboardLayoutInfo DEVICE_KEYBOARD_LAYOUT_INFO = KeyboardLayoutInfo("en-US", "qwerty"); + sp mFakePolicy; const std::unordered_map mKeyCodeMap{{KEY_0, AKEYCODE_0}, {KEY_A, AKEYCODE_A}, @@ -89,9 +113,8 @@ protected: InputMapperUnitTest::SetUp(); // set key-codes expected in tests - for (const auto& [scanCode, outKeycode] : mKeyCodeMap) { - EXPECT_CALL(mMockEventHub, mapKey(EVENTHUB_ID, scanCode, _, _, _, _, _)) - .WillRepeatedly(DoAll(SetArgPointee<4>(outKeycode), Return(NO_ERROR))); + for (const auto& [evdevCode, outKeycode] : mKeyCodeMap) { + addKeyByEvdevCode(evdevCode, outKeycode); } mFakePolicy = sp::make(); @@ -102,8 +125,79 @@ protected: mMapper = createInputMapper(*mDeviceContext, mReaderConfiguration, AINPUT_SOURCE_KEYBOARD); } + + void addKeyByEvdevCode(int32_t evdevCode, int32_t keyCode, int32_t flags = 0) { + EXPECT_CALL(mMockEventHub, mapKey(EVENTHUB_ID, evdevCode, _, _, _, _, _)) + .WillRepeatedly([=](int32_t, int32_t, int32_t, int32_t metaState, + int32_t* outKeycode, int32_t* outMetaState, + uint32_t* outFlags) { + if (outKeycode != nullptr) { + *outKeycode = keyCode; + } + if (outMetaState != nullptr) { + *outMetaState = metaState; + } + if (outFlags != nullptr) { + *outFlags = flags; + } + return NO_ERROR; + }); + } + + void addKeyByUsageCode(int32_t usageCode, int32_t keyCode, int32_t flags = 0) { + EXPECT_CALL(mMockEventHub, mapKey(EVENTHUB_ID, _, usageCode, _, _, _, _)) + .WillRepeatedly([=](int32_t, int32_t, int32_t, int32_t metaState, + int32_t* outKeycode, int32_t* outMetaState, + uint32_t* outFlags) { + if (outKeycode != nullptr) { + *outKeycode = keyCode; + } + if (outMetaState != nullptr) { + *outMetaState = metaState; + } + if (outFlags != nullptr) { + *outFlags = flags; + } + return NO_ERROR; + }); + } + + void setDisplayOrientation(ui::Rotation orientation) { + EXPECT_CALL((*mDevice), getAssociatedViewport) + .WillRepeatedly(Return(createPrimaryViewport(orientation))); + std::list args = + mMapper->reconfigure(ARBITRARY_TIME, mReaderConfiguration, + InputReaderConfiguration::Change::DISPLAY_INFO); + ASSERT_EQ(0u, args.size()); + } + + NotifyKeyArgs expectSingleKeyArg(const std::list& args) { + EXPECT_EQ(1u, args.size()); + return std::get(args.front()); + } + + void testDPadKeyRotation(int32_t originalEvdevCode, int32_t originalKeyCode, + int32_t rotatedKeyCode, ui::LogicalDisplayId displayId = DISPLAY_ID) { + std::list argsList = process(ARBITRARY_TIME, EV_KEY, originalEvdevCode, 1); + NotifyKeyArgs args = expectSingleKeyArg(argsList); + ASSERT_EQ(AKEY_EVENT_ACTION_DOWN, args.action); + ASSERT_EQ(originalEvdevCode, args.scanCode); + ASSERT_EQ(rotatedKeyCode, args.keyCode); + ASSERT_EQ(displayId, args.displayId); + + argsList = process(ARBITRARY_TIME, EV_KEY, originalEvdevCode, 0); + args = expectSingleKeyArg(argsList); + ASSERT_EQ(AKEY_EVENT_ACTION_UP, args.action); + ASSERT_EQ(originalEvdevCode, args.scanCode); + ASSERT_EQ(rotatedKeyCode, args.keyCode); + ASSERT_EQ(displayId, args.displayId); + } }; +TEST_F(KeyboardInputMapperUnitTest, GetSources) { + ASSERT_EQ(AINPUT_SOURCE_KEYBOARD, mMapper->getSources()); +} + TEST_F(KeyboardInputMapperUnitTest, KeyPressTimestampRecorded) { nsecs_t when = ARBITRARY_TIME; std::vector keyCodes{KEY_0, KEY_A, KEY_LEFTCTRL, KEY_RIGHTALT, KEY_LEFTSHIFT}; @@ -138,79 +232,17 @@ TEST_F(KeyboardInputMapperUnitTest, RepeatEventsDiscarded) { WithScanCode(KEY_0))))); } -// TODO(b/283812079): convert the tests below to use InputMapperUnitTest. - -// --- KeyboardInputMapperTest --- - -class KeyboardInputMapperTest : public InputMapperTest { -protected: - void SetUp() override { - InputMapperTest::SetUp(DEVICE_CLASSES | InputDeviceClass::KEYBOARD | - InputDeviceClass::ALPHAKEY); - } - const std::string UNIQUE_ID = "local:0"; - const KeyboardLayoutInfo DEVICE_KEYBOARD_LAYOUT_INFO = KeyboardLayoutInfo("en-US", "qwerty"); - void prepareDisplay(ui::Rotation orientation); - - void testDPadKeyRotation(KeyboardInputMapper& mapper, int32_t originalScanCode, - int32_t originalKeyCode, int32_t rotatedKeyCode, - ui::LogicalDisplayId displayId = ui::LogicalDisplayId::INVALID); -}; - -/* Similar to setDisplayInfoAndReconfigure, but pre-populates all parameters except for the - * orientation. - */ -void KeyboardInputMapperTest::prepareDisplay(ui::Rotation orientation) { - setDisplayInfoAndReconfigure(DISPLAY_ID, DISPLAY_WIDTH, DISPLAY_HEIGHT, orientation, UNIQUE_ID, - NO_PORT, ViewportType::INTERNAL); -} - -void KeyboardInputMapperTest::testDPadKeyRotation(KeyboardInputMapper& mapper, - int32_t originalScanCode, int32_t originalKeyCode, - int32_t rotatedKeyCode, - ui::LogicalDisplayId displayId) { - NotifyKeyArgs args; - - process(mapper, ARBITRARY_TIME, READ_TIME, EV_KEY, originalScanCode, 1); - ASSERT_NO_FATAL_FAILURE(mFakeListener->assertNotifyKeyWasCalled(&args)); - ASSERT_EQ(AKEY_EVENT_ACTION_DOWN, args.action); - ASSERT_EQ(originalScanCode, args.scanCode); - ASSERT_EQ(rotatedKeyCode, args.keyCode); - ASSERT_EQ(displayId, args.displayId); - - process(mapper, ARBITRARY_TIME, READ_TIME, EV_KEY, originalScanCode, 0); - ASSERT_NO_FATAL_FAILURE(mFakeListener->assertNotifyKeyWasCalled(&args)); - ASSERT_EQ(AKEY_EVENT_ACTION_UP, args.action); - ASSERT_EQ(originalScanCode, args.scanCode); - ASSERT_EQ(rotatedKeyCode, args.keyCode); - ASSERT_EQ(displayId, args.displayId); -} - -TEST_F(KeyboardInputMapperTest, GetSources) { - KeyboardInputMapper& mapper = - constructAndAddMapper(AINPUT_SOURCE_KEYBOARD); - - ASSERT_EQ(AINPUT_SOURCE_KEYBOARD, mapper.getSources()); -} - -TEST_F(KeyboardInputMapperTest, Process_SimpleKeyPress) { +TEST_F(KeyboardInputMapperUnitTest, Process_SimpleKeyPress) { const int32_t USAGE_A = 0x070004; - const int32_t USAGE_UNKNOWN = 0x07ffff; - mFakeEventHub->addKey(EVENTHUB_ID, KEY_HOME, 0, AKEYCODE_HOME, POLICY_FLAG_WAKE); - mFakeEventHub->addKey(EVENTHUB_ID, 0, USAGE_A, AKEYCODE_A, POLICY_FLAG_WAKE); - mFakeEventHub->addKey(EVENTHUB_ID, 0, KEY_NUMLOCK, AKEYCODE_NUM_LOCK, POLICY_FLAG_WAKE); - mFakeEventHub->addKey(EVENTHUB_ID, 0, KEY_CAPSLOCK, AKEYCODE_CAPS_LOCK, POLICY_FLAG_WAKE); - mFakeEventHub->addKey(EVENTHUB_ID, 0, KEY_SCROLLLOCK, AKEYCODE_SCROLL_LOCK, POLICY_FLAG_WAKE); + addKeyByEvdevCode(KEY_HOME, AKEYCODE_HOME, POLICY_FLAG_WAKE); + addKeyByUsageCode(USAGE_A, AKEYCODE_A, POLICY_FLAG_WAKE); - KeyboardInputMapper& mapper = - constructAndAddMapper(AINPUT_SOURCE_KEYBOARD); // Initial metastate is AMETA_NONE. - ASSERT_EQ(AMETA_NONE, mapper.getMetaState()); + ASSERT_EQ(AMETA_NONE, mMapper->getMetaState()); - // Key down by scan code. - process(mapper, ARBITRARY_TIME, READ_TIME, EV_KEY, KEY_HOME, 1); - NotifyKeyArgs args; - ASSERT_NO_FATAL_FAILURE(mFakeListener->assertNotifyKeyWasCalled(&args)); + // Key down by evdev code. + std::list argsList = process(ARBITRARY_TIME, EV_KEY, KEY_HOME, 1); + NotifyKeyArgs args = expectSingleKeyArg(argsList); ASSERT_EQ(DEVICE_ID, args.deviceId); ASSERT_EQ(AINPUT_SOURCE_KEYBOARD, args.source); ASSERT_EQ(ARBITRARY_TIME, args.eventTime); @@ -222,9 +254,9 @@ TEST_F(KeyboardInputMapperTest, Process_SimpleKeyPress) { ASSERT_EQ(POLICY_FLAG_WAKE, args.policyFlags); ASSERT_EQ(ARBITRARY_TIME, args.downTime); - // Key up by scan code. - process(mapper, ARBITRARY_TIME + 1, READ_TIME, EV_KEY, KEY_HOME, 0); - ASSERT_NO_FATAL_FAILURE(mFakeListener->assertNotifyKeyWasCalled(&args)); + // Key up by evdev code. + argsList = process(ARBITRARY_TIME + 1, EV_KEY, KEY_HOME, 0); + args = expectSingleKeyArg(argsList); ASSERT_EQ(DEVICE_ID, args.deviceId); ASSERT_EQ(AINPUT_SOURCE_KEYBOARD, args.source); ASSERT_EQ(ARBITRARY_TIME + 1, args.eventTime); @@ -237,9 +269,9 @@ TEST_F(KeyboardInputMapperTest, Process_SimpleKeyPress) { ASSERT_EQ(ARBITRARY_TIME, args.downTime); // Key down by usage code. - process(mapper, ARBITRARY_TIME, READ_TIME, EV_MSC, MSC_SCAN, USAGE_A); - process(mapper, ARBITRARY_TIME, READ_TIME, EV_KEY, 0, 1); - ASSERT_NO_FATAL_FAILURE(mFakeListener->assertNotifyKeyWasCalled(&args)); + argsList = process(ARBITRARY_TIME, EV_MSC, MSC_SCAN, USAGE_A); + argsList += process(ARBITRARY_TIME, EV_KEY, 0, 1); + args = expectSingleKeyArg(argsList); ASSERT_EQ(DEVICE_ID, args.deviceId); ASSERT_EQ(AINPUT_SOURCE_KEYBOARD, args.source); ASSERT_EQ(ARBITRARY_TIME, args.eventTime); @@ -252,9 +284,9 @@ TEST_F(KeyboardInputMapperTest, Process_SimpleKeyPress) { ASSERT_EQ(ARBITRARY_TIME, args.downTime); // Key up by usage code. - process(mapper, ARBITRARY_TIME, READ_TIME, EV_MSC, MSC_SCAN, USAGE_A); - process(mapper, ARBITRARY_TIME + 1, READ_TIME, EV_KEY, 0, 0); - ASSERT_NO_FATAL_FAILURE(mFakeListener->assertNotifyKeyWasCalled(&args)); + argsList = process(ARBITRARY_TIME, EV_MSC, MSC_SCAN, USAGE_A); + argsList += process(ARBITRARY_TIME + 1, EV_KEY, 0, 0); + args = expectSingleKeyArg(argsList); ASSERT_EQ(DEVICE_ID, args.deviceId); ASSERT_EQ(AINPUT_SOURCE_KEYBOARD, args.source); ASSERT_EQ(ARBITRARY_TIME + 1, args.eventTime); @@ -265,11 +297,17 @@ TEST_F(KeyboardInputMapperTest, Process_SimpleKeyPress) { ASSERT_EQ(AKEY_EVENT_FLAG_FROM_SYSTEM, args.flags); ASSERT_EQ(POLICY_FLAG_WAKE, args.policyFlags); ASSERT_EQ(ARBITRARY_TIME, args.downTime); +} + +TEST_F(KeyboardInputMapperUnitTest, Process_UnknownKey) { + const int32_t USAGE_UNKNOWN = 0x07ffff; + EXPECT_CALL(mMockEventHub, mapKey(EVENTHUB_ID, KEY_UNKNOWN, USAGE_UNKNOWN, _, _, _, _)) + .WillRepeatedly(Return(NAME_NOT_FOUND)); // Key down with unknown scan code or usage code. - process(mapper, ARBITRARY_TIME, READ_TIME, EV_MSC, MSC_SCAN, USAGE_UNKNOWN); - process(mapper, ARBITRARY_TIME, READ_TIME, EV_KEY, KEY_UNKNOWN, 1); - ASSERT_NO_FATAL_FAILURE(mFakeListener->assertNotifyKeyWasCalled(&args)); + std::list argsList = process(ARBITRARY_TIME, EV_MSC, MSC_SCAN, USAGE_UNKNOWN); + argsList += process(ARBITRARY_TIME, EV_KEY, KEY_UNKNOWN, 1); + NotifyKeyArgs args = expectSingleKeyArg(argsList); ASSERT_EQ(DEVICE_ID, args.deviceId); ASSERT_EQ(AINPUT_SOURCE_KEYBOARD, args.source); ASSERT_EQ(ARBITRARY_TIME, args.eventTime); @@ -282,9 +320,9 @@ TEST_F(KeyboardInputMapperTest, Process_SimpleKeyPress) { ASSERT_EQ(ARBITRARY_TIME, args.downTime); // Key up with unknown scan code or usage code. - process(mapper, ARBITRARY_TIME, READ_TIME, EV_MSC, MSC_SCAN, USAGE_UNKNOWN); - process(mapper, ARBITRARY_TIME + 1, READ_TIME, EV_KEY, KEY_UNKNOWN, 0); - ASSERT_NO_FATAL_FAILURE(mFakeListener->assertNotifyKeyWasCalled(&args)); + argsList = process(ARBITRARY_TIME, EV_MSC, MSC_SCAN, USAGE_UNKNOWN); + argsList += process(ARBITRARY_TIME + 1, EV_KEY, KEY_UNKNOWN, 0); + args = expectSingleKeyArg(argsList); ASSERT_EQ(DEVICE_ID, args.deviceId); ASSERT_EQ(AINPUT_SOURCE_KEYBOARD, args.source); ASSERT_EQ(ARBITRARY_TIME + 1, args.eventTime); @@ -297,348 +335,425 @@ TEST_F(KeyboardInputMapperTest, Process_SimpleKeyPress) { ASSERT_EQ(ARBITRARY_TIME, args.downTime); } -TEST_F(KeyboardInputMapperTest, Process_KeyRemapping) { - mFakeEventHub->addKey(EVENTHUB_ID, KEY_A, 0, AKEYCODE_A, 0); - mFakeEventHub->addKey(EVENTHUB_ID, KEY_B, 0, AKEYCODE_B, 0); - - KeyboardInputMapper& mapper = - constructAndAddMapper(AINPUT_SOURCE_KEYBOARD); - - mFakeEventHub->setKeyRemapping(EVENTHUB_ID, {{AKEYCODE_A, AKEYCODE_B}}); - // Key down by scan code. - process(mapper, ARBITRARY_TIME, READ_TIME, EV_KEY, KEY_A, 1); - NotifyKeyArgs args; - ASSERT_NO_FATAL_FAILURE(mFakeListener->assertNotifyKeyWasCalled(&args)); - ASSERT_EQ(AKEYCODE_B, args.keyCode); - - // Key up by scan code. - process(mapper, ARBITRARY_TIME + 1, READ_TIME, EV_KEY, KEY_A, 0); - ASSERT_NO_FATAL_FAILURE(mFakeListener->assertNotifyKeyWasCalled(&args)); - ASSERT_EQ(AKEYCODE_B, args.keyCode); -} - /** * Ensure that the readTime is set to the time when the EV_KEY is received. */ -TEST_F(KeyboardInputMapperTest, Process_SendsReadTime) { - mFakeEventHub->addKey(EVENTHUB_ID, KEY_HOME, 0, AKEYCODE_HOME, POLICY_FLAG_WAKE); - - KeyboardInputMapper& mapper = - constructAndAddMapper(AINPUT_SOURCE_KEYBOARD); - NotifyKeyArgs args; +TEST_F(KeyboardInputMapperUnitTest, Process_SendsReadTime) { + addKeyByEvdevCode(KEY_HOME, AKEYCODE_HOME); // Key down - process(mapper, ARBITRARY_TIME, /*readTime=*/12, EV_KEY, KEY_HOME, 1); - ASSERT_NO_FATAL_FAILURE(mFakeListener->assertNotifyKeyWasCalled(&args)); - ASSERT_EQ(12, args.readTime); + std::list argsList = process(ARBITRARY_TIME, /*readTime=*/12, EV_KEY, KEY_HOME, 1); + ASSERT_EQ(12, expectSingleKeyArg(argsList).readTime); // Key up - process(mapper, ARBITRARY_TIME, /*readTime=*/15, EV_KEY, KEY_HOME, 1); - ASSERT_NO_FATAL_FAILURE(mFakeListener->assertNotifyKeyWasCalled(&args)); - ASSERT_EQ(15, args.readTime); + argsList = process(ARBITRARY_TIME, /*readTime=*/15, EV_KEY, KEY_HOME, 1); + ASSERT_EQ(15, expectSingleKeyArg(argsList).readTime); } -TEST_F(KeyboardInputMapperTest, Process_ShouldUpdateMetaState) { - mFakeEventHub->addKey(EVENTHUB_ID, KEY_LEFTSHIFT, 0, AKEYCODE_SHIFT_LEFT, 0); - mFakeEventHub->addKey(EVENTHUB_ID, KEY_A, 0, AKEYCODE_A, 0); - mFakeEventHub->addKey(EVENTHUB_ID, 0, KEY_NUMLOCK, AKEYCODE_NUM_LOCK, 0); - mFakeEventHub->addKey(EVENTHUB_ID, 0, KEY_CAPSLOCK, AKEYCODE_CAPS_LOCK, 0); - mFakeEventHub->addKey(EVENTHUB_ID, 0, KEY_SCROLLLOCK, AKEYCODE_SCROLL_LOCK, 0); +TEST_F(KeyboardInputMapperUnitTest, Process_ShouldUpdateMetaState) { + addKeyByEvdevCode(KEY_LEFTSHIFT, AKEYCODE_SHIFT_LEFT); + addKeyByEvdevCode(KEY_A, AKEYCODE_A); - KeyboardInputMapper& mapper = - constructAndAddMapper(AINPUT_SOURCE_KEYBOARD); + EXPECT_CALL(mMockInputReaderContext, updateGlobalMetaState()).Times(2); // Initial metastate is AMETA_NONE. - ASSERT_EQ(AMETA_NONE, mapper.getMetaState()); + ASSERT_EQ(AMETA_NONE, mMapper->getMetaState()); // Metakey down. - process(mapper, ARBITRARY_TIME, READ_TIME, EV_KEY, KEY_LEFTSHIFT, 1); - NotifyKeyArgs args; - ASSERT_NO_FATAL_FAILURE(mFakeListener->assertNotifyKeyWasCalled(&args)); - ASSERT_EQ(AMETA_SHIFT_LEFT_ON | AMETA_SHIFT_ON, args.metaState); - ASSERT_EQ(AMETA_SHIFT_LEFT_ON | AMETA_SHIFT_ON, mapper.getMetaState()); - ASSERT_NO_FATAL_FAILURE(mReader->getContext()->assertUpdateGlobalMetaStateWasCalled()); + std::list argsList = process(ARBITRARY_TIME, EV_KEY, KEY_LEFTSHIFT, 1); + ASSERT_EQ(AMETA_SHIFT_LEFT_ON | AMETA_SHIFT_ON, expectSingleKeyArg(argsList).metaState); + ASSERT_EQ(AMETA_SHIFT_LEFT_ON | AMETA_SHIFT_ON, mMapper->getMetaState()); // Key down. - process(mapper, ARBITRARY_TIME + 1, READ_TIME, EV_KEY, KEY_A, 1); - ASSERT_NO_FATAL_FAILURE(mFakeListener->assertNotifyKeyWasCalled(&args)); - ASSERT_EQ(AMETA_SHIFT_LEFT_ON | AMETA_SHIFT_ON, args.metaState); - ASSERT_EQ(AMETA_SHIFT_LEFT_ON | AMETA_SHIFT_ON, mapper.getMetaState()); + argsList = process(ARBITRARY_TIME + 1, EV_KEY, KEY_A, 1); + ASSERT_EQ(AMETA_SHIFT_LEFT_ON | AMETA_SHIFT_ON, expectSingleKeyArg(argsList).metaState); + ASSERT_EQ(AMETA_SHIFT_LEFT_ON | AMETA_SHIFT_ON, mMapper->getMetaState()); // Key up. - process(mapper, ARBITRARY_TIME + 2, READ_TIME, EV_KEY, KEY_A, 0); - ASSERT_NO_FATAL_FAILURE(mFakeListener->assertNotifyKeyWasCalled(&args)); - ASSERT_EQ(AMETA_SHIFT_LEFT_ON | AMETA_SHIFT_ON, args.metaState); - ASSERT_EQ(AMETA_SHIFT_LEFT_ON | AMETA_SHIFT_ON, mapper.getMetaState()); + argsList = process(ARBITRARY_TIME + 2, EV_KEY, KEY_A, 0); + ASSERT_EQ(AMETA_SHIFT_LEFT_ON | AMETA_SHIFT_ON, expectSingleKeyArg(argsList).metaState); + ASSERT_EQ(AMETA_SHIFT_LEFT_ON | AMETA_SHIFT_ON, mMapper->getMetaState()); // Metakey up. - process(mapper, ARBITRARY_TIME + 3, READ_TIME, EV_KEY, KEY_LEFTSHIFT, 0); - ASSERT_NO_FATAL_FAILURE(mFakeListener->assertNotifyKeyWasCalled(&args)); - ASSERT_EQ(AMETA_NONE, args.metaState); - ASSERT_EQ(AMETA_NONE, mapper.getMetaState()); - ASSERT_NO_FATAL_FAILURE(mReader->getContext()->assertUpdateGlobalMetaStateWasCalled()); + argsList = process(ARBITRARY_TIME + 3, EV_KEY, KEY_LEFTSHIFT, 0); + ASSERT_EQ(AMETA_NONE, expectSingleKeyArg(argsList).metaState); + ASSERT_EQ(AMETA_NONE, mMapper->getMetaState()); } -TEST_F(KeyboardInputMapperTest, Process_WhenNotOrientationAware_ShouldNotRotateDPad) { - mFakeEventHub->addKey(EVENTHUB_ID, KEY_UP, 0, AKEYCODE_DPAD_UP, 0); - mFakeEventHub->addKey(EVENTHUB_ID, KEY_RIGHT, 0, AKEYCODE_DPAD_RIGHT, 0); - mFakeEventHub->addKey(EVENTHUB_ID, KEY_DOWN, 0, AKEYCODE_DPAD_DOWN, 0); - mFakeEventHub->addKey(EVENTHUB_ID, KEY_LEFT, 0, AKEYCODE_DPAD_LEFT, 0); +TEST_F(KeyboardInputMapperUnitTest, Process_WhenNotOrientationAware_ShouldNotRotateDPad) { + addKeyByEvdevCode(KEY_UP, AKEYCODE_DPAD_UP); + addKeyByEvdevCode(KEY_RIGHT, AKEYCODE_DPAD_RIGHT); + addKeyByEvdevCode(KEY_DOWN, AKEYCODE_DPAD_DOWN); + addKeyByEvdevCode(KEY_LEFT, AKEYCODE_DPAD_LEFT); - KeyboardInputMapper& mapper = - constructAndAddMapper(AINPUT_SOURCE_KEYBOARD); - - prepareDisplay(ui::ROTATION_90); - ASSERT_NO_FATAL_FAILURE(testDPadKeyRotation(mapper, - KEY_UP, AKEYCODE_DPAD_UP, AKEYCODE_DPAD_UP)); - ASSERT_NO_FATAL_FAILURE(testDPadKeyRotation(mapper, - KEY_RIGHT, AKEYCODE_DPAD_RIGHT, AKEYCODE_DPAD_RIGHT)); - ASSERT_NO_FATAL_FAILURE(testDPadKeyRotation(mapper, - KEY_DOWN, AKEYCODE_DPAD_DOWN, AKEYCODE_DPAD_DOWN)); - ASSERT_NO_FATAL_FAILURE(testDPadKeyRotation(mapper, - KEY_LEFT, AKEYCODE_DPAD_LEFT, AKEYCODE_DPAD_LEFT)); + setDisplayOrientation(ui::Rotation::Rotation90); + ASSERT_NO_FATAL_FAILURE(testDPadKeyRotation(KEY_UP, AKEYCODE_DPAD_UP, AKEYCODE_DPAD_UP)); + ASSERT_NO_FATAL_FAILURE( + testDPadKeyRotation(KEY_RIGHT, AKEYCODE_DPAD_RIGHT, AKEYCODE_DPAD_RIGHT)); + ASSERT_NO_FATAL_FAILURE(testDPadKeyRotation(KEY_DOWN, AKEYCODE_DPAD_DOWN, AKEYCODE_DPAD_DOWN)); + ASSERT_NO_FATAL_FAILURE(testDPadKeyRotation(KEY_LEFT, AKEYCODE_DPAD_LEFT, AKEYCODE_DPAD_LEFT)); } -TEST_F(KeyboardInputMapperTest, Process_WhenOrientationAware_ShouldRotateDPad) { - mFakeEventHub->addKey(EVENTHUB_ID, KEY_UP, 0, AKEYCODE_DPAD_UP, 0); - mFakeEventHub->addKey(EVENTHUB_ID, KEY_RIGHT, 0, AKEYCODE_DPAD_RIGHT, 0); - mFakeEventHub->addKey(EVENTHUB_ID, KEY_DOWN, 0, AKEYCODE_DPAD_DOWN, 0); - mFakeEventHub->addKey(EVENTHUB_ID, KEY_LEFT, 0, AKEYCODE_DPAD_LEFT, 0); +TEST_F(KeyboardInputMapperUnitTest, Process_WhenOrientationAware_ShouldRotateDPad) { + addKeyByEvdevCode(KEY_UP, AKEYCODE_DPAD_UP); + addKeyByEvdevCode(KEY_RIGHT, AKEYCODE_DPAD_RIGHT); + addKeyByEvdevCode(KEY_DOWN, AKEYCODE_DPAD_DOWN); + addKeyByEvdevCode(KEY_LEFT, AKEYCODE_DPAD_LEFT); - addConfigurationProperty("keyboard.orientationAware", "1"); - KeyboardInputMapper& mapper = - constructAndAddMapper(AINPUT_SOURCE_KEYBOARD); + mPropertyMap.addProperty("keyboard.orientationAware", "1"); + mMapper = createInputMapper(*mDeviceContext, mReaderConfiguration, + AINPUT_SOURCE_KEYBOARD); + setDisplayOrientation(ui::ROTATION_0); - prepareDisplay(ui::ROTATION_0); + ASSERT_NO_FATAL_FAILURE(testDPadKeyRotation(KEY_UP, AKEYCODE_DPAD_UP, AKEYCODE_DPAD_UP)); ASSERT_NO_FATAL_FAILURE( - testDPadKeyRotation(mapper, KEY_UP, AKEYCODE_DPAD_UP, AKEYCODE_DPAD_UP, DISPLAY_ID)); - ASSERT_NO_FATAL_FAILURE(testDPadKeyRotation(mapper, KEY_RIGHT, AKEYCODE_DPAD_RIGHT, - AKEYCODE_DPAD_RIGHT, DISPLAY_ID)); - ASSERT_NO_FATAL_FAILURE(testDPadKeyRotation(mapper, KEY_DOWN, AKEYCODE_DPAD_DOWN, - AKEYCODE_DPAD_DOWN, DISPLAY_ID)); - ASSERT_NO_FATAL_FAILURE(testDPadKeyRotation(mapper, KEY_LEFT, AKEYCODE_DPAD_LEFT, - AKEYCODE_DPAD_LEFT, DISPLAY_ID)); - - clearViewports(); - prepareDisplay(ui::ROTATION_90); + testDPadKeyRotation(KEY_RIGHT, AKEYCODE_DPAD_RIGHT, AKEYCODE_DPAD_RIGHT)); + ASSERT_NO_FATAL_FAILURE(testDPadKeyRotation(KEY_DOWN, AKEYCODE_DPAD_DOWN, AKEYCODE_DPAD_DOWN)); + ASSERT_NO_FATAL_FAILURE(testDPadKeyRotation(KEY_LEFT, AKEYCODE_DPAD_LEFT, AKEYCODE_DPAD_LEFT)); + + setDisplayOrientation(ui::ROTATION_90); + ASSERT_NO_FATAL_FAILURE(testDPadKeyRotation(KEY_UP, AKEYCODE_DPAD_UP, AKEYCODE_DPAD_LEFT)); + ASSERT_NO_FATAL_FAILURE(testDPadKeyRotation(KEY_RIGHT, AKEYCODE_DPAD_RIGHT, AKEYCODE_DPAD_UP)); + ASSERT_NO_FATAL_FAILURE(testDPadKeyRotation(KEY_DOWN, AKEYCODE_DPAD_DOWN, AKEYCODE_DPAD_RIGHT)); + ASSERT_NO_FATAL_FAILURE(testDPadKeyRotation(KEY_LEFT, AKEYCODE_DPAD_LEFT, AKEYCODE_DPAD_DOWN)); + + setDisplayOrientation(ui::ROTATION_180); + ASSERT_NO_FATAL_FAILURE(testDPadKeyRotation(KEY_UP, AKEYCODE_DPAD_UP, AKEYCODE_DPAD_DOWN)); ASSERT_NO_FATAL_FAILURE( - testDPadKeyRotation(mapper, KEY_UP, AKEYCODE_DPAD_UP, AKEYCODE_DPAD_LEFT, DISPLAY_ID)); - ASSERT_NO_FATAL_FAILURE(testDPadKeyRotation(mapper, KEY_RIGHT, AKEYCODE_DPAD_RIGHT, - AKEYCODE_DPAD_UP, DISPLAY_ID)); - ASSERT_NO_FATAL_FAILURE(testDPadKeyRotation(mapper, KEY_DOWN, AKEYCODE_DPAD_DOWN, - AKEYCODE_DPAD_RIGHT, DISPLAY_ID)); - ASSERT_NO_FATAL_FAILURE(testDPadKeyRotation(mapper, KEY_LEFT, AKEYCODE_DPAD_LEFT, - AKEYCODE_DPAD_DOWN, DISPLAY_ID)); + testDPadKeyRotation(KEY_RIGHT, AKEYCODE_DPAD_RIGHT, AKEYCODE_DPAD_LEFT)); + ASSERT_NO_FATAL_FAILURE(testDPadKeyRotation(KEY_DOWN, AKEYCODE_DPAD_DOWN, AKEYCODE_DPAD_UP)); + ASSERT_NO_FATAL_FAILURE(testDPadKeyRotation(KEY_LEFT, AKEYCODE_DPAD_LEFT, AKEYCODE_DPAD_RIGHT)); - clearViewports(); - prepareDisplay(ui::ROTATION_180); + setDisplayOrientation(ui::ROTATION_270); + ASSERT_NO_FATAL_FAILURE(testDPadKeyRotation(KEY_UP, AKEYCODE_DPAD_UP, AKEYCODE_DPAD_RIGHT)); ASSERT_NO_FATAL_FAILURE( - testDPadKeyRotation(mapper, KEY_UP, AKEYCODE_DPAD_UP, AKEYCODE_DPAD_DOWN, DISPLAY_ID)); - ASSERT_NO_FATAL_FAILURE(testDPadKeyRotation(mapper, KEY_RIGHT, AKEYCODE_DPAD_RIGHT, - AKEYCODE_DPAD_LEFT, DISPLAY_ID)); - ASSERT_NO_FATAL_FAILURE(testDPadKeyRotation(mapper, KEY_DOWN, AKEYCODE_DPAD_DOWN, - AKEYCODE_DPAD_UP, DISPLAY_ID)); - ASSERT_NO_FATAL_FAILURE(testDPadKeyRotation(mapper, KEY_LEFT, AKEYCODE_DPAD_LEFT, - AKEYCODE_DPAD_RIGHT, DISPLAY_ID)); - - clearViewports(); - prepareDisplay(ui::ROTATION_270); - ASSERT_NO_FATAL_FAILURE( - testDPadKeyRotation(mapper, KEY_UP, AKEYCODE_DPAD_UP, AKEYCODE_DPAD_RIGHT, DISPLAY_ID)); - ASSERT_NO_FATAL_FAILURE(testDPadKeyRotation(mapper, KEY_RIGHT, AKEYCODE_DPAD_RIGHT, - AKEYCODE_DPAD_DOWN, DISPLAY_ID)); - ASSERT_NO_FATAL_FAILURE(testDPadKeyRotation(mapper, KEY_DOWN, AKEYCODE_DPAD_DOWN, - AKEYCODE_DPAD_LEFT, DISPLAY_ID)); - ASSERT_NO_FATAL_FAILURE(testDPadKeyRotation(mapper, KEY_LEFT, AKEYCODE_DPAD_LEFT, - AKEYCODE_DPAD_UP, DISPLAY_ID)); + testDPadKeyRotation(KEY_RIGHT, AKEYCODE_DPAD_RIGHT, AKEYCODE_DPAD_DOWN)); + ASSERT_NO_FATAL_FAILURE(testDPadKeyRotation(KEY_DOWN, AKEYCODE_DPAD_DOWN, AKEYCODE_DPAD_LEFT)); + ASSERT_NO_FATAL_FAILURE(testDPadKeyRotation(KEY_LEFT, AKEYCODE_DPAD_LEFT, AKEYCODE_DPAD_UP)); // Special case: if orientation changes while key is down, we still emit the same keycode // in the key up as we did in the key down. - NotifyKeyArgs args; - clearViewports(); - prepareDisplay(ui::ROTATION_270); - process(mapper, ARBITRARY_TIME, READ_TIME, EV_KEY, KEY_UP, 1); - ASSERT_NO_FATAL_FAILURE(mFakeListener->assertNotifyKeyWasCalled(&args)); + setDisplayOrientation(ui::ROTATION_270); + std::list argsList = process(ARBITRARY_TIME, EV_KEY, KEY_UP, 1); + NotifyKeyArgs args = expectSingleKeyArg(argsList); ASSERT_EQ(AKEY_EVENT_ACTION_DOWN, args.action); ASSERT_EQ(KEY_UP, args.scanCode); ASSERT_EQ(AKEYCODE_DPAD_RIGHT, args.keyCode); - clearViewports(); - prepareDisplay(ui::ROTATION_180); - process(mapper, ARBITRARY_TIME, READ_TIME, EV_KEY, KEY_UP, 0); - ASSERT_NO_FATAL_FAILURE(mFakeListener->assertNotifyKeyWasCalled(&args)); + setDisplayOrientation(ui::ROTATION_180); + argsList = process(ARBITRARY_TIME, EV_KEY, KEY_UP, 0); + args = expectSingleKeyArg(argsList); ASSERT_EQ(AKEY_EVENT_ACTION_UP, args.action); ASSERT_EQ(KEY_UP, args.scanCode); ASSERT_EQ(AKEYCODE_DPAD_RIGHT, args.keyCode); } -TEST_F(KeyboardInputMapperTest, DisplayIdConfigurationChange_NotOrientationAware) { +TEST_F(KeyboardInputMapperUnitTest, DisplayIdConfigurationChange_NotOrientationAware) { // If the keyboard is not orientation aware, // key events should not be associated with a specific display id - mFakeEventHub->addKey(EVENTHUB_ID, KEY_UP, 0, AKEYCODE_DPAD_UP, 0); - - KeyboardInputMapper& mapper = - constructAndAddMapper(AINPUT_SOURCE_KEYBOARD); - NotifyKeyArgs args; + addKeyByEvdevCode(KEY_UP, AKEYCODE_DPAD_UP); // Display id should be LogicalDisplayId::INVALID without any display configuration. - process(mapper, ARBITRARY_TIME, READ_TIME, EV_KEY, KEY_UP, 1); - ASSERT_NO_FATAL_FAILURE(mFakeListener->assertNotifyKeyWasCalled(&args)); - process(mapper, ARBITRARY_TIME, READ_TIME, EV_KEY, KEY_UP, 0); - ASSERT_NO_FATAL_FAILURE(mFakeListener->assertNotifyKeyWasCalled(&args)); - ASSERT_EQ(ui::LogicalDisplayId::INVALID, args.displayId); - - prepareDisplay(ui::ROTATION_0); - process(mapper, ARBITRARY_TIME, READ_TIME, EV_KEY, KEY_UP, 1); - ASSERT_NO_FATAL_FAILURE(mFakeListener->assertNotifyKeyWasCalled(&args)); - process(mapper, ARBITRARY_TIME, READ_TIME, EV_KEY, KEY_UP, 0); - ASSERT_NO_FATAL_FAILURE(mFakeListener->assertNotifyKeyWasCalled(&args)); - ASSERT_EQ(ui::LogicalDisplayId::INVALID, args.displayId); + std::list argsList = process(ARBITRARY_TIME, EV_KEY, KEY_UP, 1); + ASSERT_GT(argsList.size(), 0u); + argsList = process(ARBITRARY_TIME, EV_KEY, KEY_UP, 0); + ASSERT_GT(argsList.size(), 0u); + ASSERT_EQ(ui::LogicalDisplayId::INVALID, std::get(argsList.front()).displayId); } -TEST_F(KeyboardInputMapperTest, DisplayIdConfigurationChange_OrientationAware) { +TEST_F(KeyboardInputMapperUnitTest, DisplayIdConfigurationChange_OrientationAware) { // If the keyboard is orientation aware, // key events should be associated with the internal viewport - mFakeEventHub->addKey(EVENTHUB_ID, KEY_UP, 0, AKEYCODE_DPAD_UP, 0); + addKeyByEvdevCode(KEY_UP, AKEYCODE_DPAD_UP); - addConfigurationProperty("keyboard.orientationAware", "1"); - KeyboardInputMapper& mapper = - constructAndAddMapper(AINPUT_SOURCE_KEYBOARD); - NotifyKeyArgs args; + mPropertyMap.addProperty("keyboard.orientationAware", "1"); + mMapper = createInputMapper(*mDeviceContext, mReaderConfiguration, + AINPUT_SOURCE_KEYBOARD); // Display id should be LogicalDisplayId::INVALID without any display configuration. // ^--- already checked by the previous test - setDisplayInfoAndReconfigure(DISPLAY_ID, DISPLAY_WIDTH, DISPLAY_HEIGHT, ui::ROTATION_0, - UNIQUE_ID, NO_PORT, ViewportType::INTERNAL); - process(mapper, ARBITRARY_TIME, READ_TIME, EV_KEY, KEY_UP, 1); - ASSERT_NO_FATAL_FAILURE(mFakeListener->assertNotifyKeyWasCalled(&args)); - process(mapper, ARBITRARY_TIME, READ_TIME, EV_KEY, KEY_UP, 0); - ASSERT_NO_FATAL_FAILURE(mFakeListener->assertNotifyKeyWasCalled(&args)); - ASSERT_EQ(DISPLAY_ID, args.displayId); + setDisplayOrientation(ui::ROTATION_0); + std::list argsList = process(ARBITRARY_TIME, EV_KEY, KEY_UP, 1); + ASSERT_GT(argsList.size(), 0u); + argsList = process(ARBITRARY_TIME, EV_KEY, KEY_UP, 0); + ASSERT_GT(argsList.size(), 0u); + ASSERT_EQ(DISPLAY_ID, std::get(argsList.front()).displayId); constexpr ui::LogicalDisplayId newDisplayId = ui::LogicalDisplayId{2}; - clearViewports(); - setDisplayInfoAndReconfigure(newDisplayId, DISPLAY_WIDTH, DISPLAY_HEIGHT, ui::ROTATION_0, - UNIQUE_ID, NO_PORT, ViewportType::INTERNAL); - process(mapper, ARBITRARY_TIME, READ_TIME, EV_KEY, KEY_UP, 1); - ASSERT_NO_FATAL_FAILURE(mFakeListener->assertNotifyKeyWasCalled(&args)); - process(mapper, ARBITRARY_TIME, READ_TIME, EV_KEY, KEY_UP, 0); - ASSERT_NO_FATAL_FAILURE(mFakeListener->assertNotifyKeyWasCalled(&args)); - ASSERT_EQ(newDisplayId, args.displayId); + DisplayViewport newViewport = createPrimaryViewport(ui::ROTATION_0); + newViewport.displayId = newDisplayId; + EXPECT_CALL((*mDevice), getAssociatedViewport).WillRepeatedly(Return(newViewport)); + argsList = mMapper->reconfigure(ARBITRARY_TIME, mReaderConfiguration, + InputReaderConfiguration::Change::DISPLAY_INFO); + ASSERT_EQ(0u, argsList.size()); + argsList = process(ARBITRARY_TIME, EV_KEY, KEY_UP, 1); + ASSERT_GT(argsList.size(), 0u); + argsList = process(ARBITRARY_TIME, EV_KEY, KEY_UP, 0); + ASSERT_GT(argsList.size(), 0u); + ASSERT_EQ(newDisplayId, std::get(argsList.front()).displayId); } -TEST_F(KeyboardInputMapperTest, GetKeyCodeState) { - KeyboardInputMapper& mapper = - constructAndAddMapper(AINPUT_SOURCE_KEYBOARD); - - mFakeEventHub->setKeyCodeState(EVENTHUB_ID, AKEYCODE_A, 1); - ASSERT_EQ(1, mapper.getKeyCodeState(AINPUT_SOURCE_ANY, AKEYCODE_A)); +TEST_F(KeyboardInputMapperUnitTest, GetKeyCodeState) { + EXPECT_CALL(mMockEventHub, getKeyCodeState(EVENTHUB_ID, AKEYCODE_A)) + .WillRepeatedly(Return(AKEY_STATE_DOWN)); + ASSERT_EQ(AKEY_STATE_DOWN, mMapper->getKeyCodeState(AINPUT_SOURCE_ANY, AKEYCODE_A)); - mFakeEventHub->setKeyCodeState(EVENTHUB_ID, AKEYCODE_A, 0); - ASSERT_EQ(0, mapper.getKeyCodeState(AINPUT_SOURCE_ANY, AKEYCODE_A)); + EXPECT_CALL(mMockEventHub, getKeyCodeState(EVENTHUB_ID, AKEYCODE_A)) + .WillRepeatedly(Return(AKEY_STATE_UP)); + ASSERT_EQ(AKEY_STATE_UP, mMapper->getKeyCodeState(AINPUT_SOURCE_ANY, AKEYCODE_A)); } -TEST_F(KeyboardInputMapperTest, GetKeyCodeForKeyLocation) { - KeyboardInputMapper& mapper = - constructAndAddMapper(AINPUT_SOURCE_KEYBOARD); - - mFakeEventHub->addKeyCodeMapping(EVENTHUB_ID, AKEYCODE_Y, AKEYCODE_Z); - ASSERT_EQ(AKEYCODE_Z, mapper.getKeyCodeForKeyLocation(AKEYCODE_Y)) - << "If a mapping is available, the result is equal to the mapping"; +TEST_F(KeyboardInputMapperUnitTest, GetKeyCodeForKeyLocation) { + EXPECT_CALL(mMockEventHub, getKeyCodeForKeyLocation(EVENTHUB_ID, _)) + .WillRepeatedly(ReturnArg<1>()); + EXPECT_CALL(mMockEventHub, getKeyCodeForKeyLocation(EVENTHUB_ID, AKEYCODE_Y)) + .WillRepeatedly(Return(AKEYCODE_Z)); + ASSERT_EQ(AKEYCODE_Z, mMapper->getKeyCodeForKeyLocation(AKEYCODE_Y)) + << "If a mapping is available, the result is equal to the mapping"; - ASSERT_EQ(AKEYCODE_A, mapper.getKeyCodeForKeyLocation(AKEYCODE_A)) - << "If no mapping is available, the result is the key location"; + ASSERT_EQ(AKEYCODE_A, mMapper->getKeyCodeForKeyLocation(AKEYCODE_A)) + << "If no mapping is available, the result is the key location"; } -TEST_F(KeyboardInputMapperTest, GetScanCodeState) { - KeyboardInputMapper& mapper = - constructAndAddMapper(AINPUT_SOURCE_KEYBOARD); +TEST_F(KeyboardInputMapperUnitTest, GetScanCodeState) { + EXPECT_CALL(mMockEventHub, getScanCodeState(EVENTHUB_ID, KEY_A)) + .WillRepeatedly(Return(AKEY_STATE_DOWN)); + ASSERT_EQ(AKEY_STATE_DOWN, mMapper->getScanCodeState(AINPUT_SOURCE_ANY, KEY_A)); - mFakeEventHub->setScanCodeState(EVENTHUB_ID, KEY_A, 1); - ASSERT_EQ(1, mapper.getScanCodeState(AINPUT_SOURCE_ANY, KEY_A)); - - mFakeEventHub->setScanCodeState(EVENTHUB_ID, KEY_A, 0); - ASSERT_EQ(0, mapper.getScanCodeState(AINPUT_SOURCE_ANY, KEY_A)); + EXPECT_CALL(mMockEventHub, getScanCodeState(EVENTHUB_ID, KEY_A)) + .WillRepeatedly(Return(AKEY_STATE_UP)); + ASSERT_EQ(AKEY_STATE_UP, mMapper->getScanCodeState(AINPUT_SOURCE_ANY, KEY_A)); } -TEST_F(KeyboardInputMapperTest, MarkSupportedKeyCodes) { - KeyboardInputMapper& mapper = - constructAndAddMapper(AINPUT_SOURCE_KEYBOARD); +TEST_F(KeyboardInputMapperUnitTest, Process_LockedKeysShouldToggleMetaStateAndLeds) { + EXPECT_CALL(mMockEventHub, + hasLed(EVENTHUB_ID, AnyOf(LED_CAPSL, LED_NUML, LED_SCROLLL /*NOTYPO*/))) + .WillRepeatedly(Return(true)); + bool capsLockLed = true; // Initially on + bool numLockLed = false; // Initially off + bool scrollLockLed = false; // Initially off + EXPECT_CALL(mMockEventHub, setLedState(EVENTHUB_ID, LED_CAPSL, _)) + .WillRepeatedly(SaveArg<2>(&capsLockLed)); + EXPECT_CALL(mMockEventHub, setLedState(EVENTHUB_ID, LED_NUML, _)) + .WillRepeatedly(SaveArg<2>(&numLockLed)); + EXPECT_CALL(mMockEventHub, setLedState(EVENTHUB_ID, LED_SCROLLL /*NOTYPO*/, _)) + .WillRepeatedly(SaveArg<2>(&scrollLockLed)); + addKeyByEvdevCode(KEY_CAPSLOCK, AKEYCODE_CAPS_LOCK); + addKeyByEvdevCode(KEY_NUMLOCK, AKEYCODE_NUM_LOCK); + addKeyByEvdevCode(KEY_SCROLLLOCK, AKEYCODE_SCROLL_LOCK); + + // In real operation, mappers pass new LED states to InputReader (via the context), which then + // calls back to the mappers to apply that state. Mimic the same thing here with mocks. + int32_t ledMetaState; + EXPECT_CALL(mMockInputReaderContext, updateLedMetaState(_)) + .WillRepeatedly([&](int32_t newState) { + ledMetaState = newState; + mMapper->updateLedState(false); + }); + EXPECT_CALL(mMockInputReaderContext, getLedMetaState()) + .WillRepeatedly(testing::ReturnPointee(&ledMetaState)); + + ASSERT_THAT(mMapper->reset(ARBITRARY_TIME), IsEmpty()); - mFakeEventHub->addKey(EVENTHUB_ID, KEY_A, 0, AKEYCODE_A, 0); - - uint8_t flags[2] = { 0, 0 }; - ASSERT_TRUE(mapper.markSupportedKeyCodes(AINPUT_SOURCE_ANY, {AKEYCODE_A, AKEYCODE_B}, flags)); - ASSERT_TRUE(flags[0]); - ASSERT_FALSE(flags[1]); -} - -TEST_F(KeyboardInputMapperTest, Process_LockedKeysShouldToggleMetaStateAndLeds) { - mFakeEventHub->addLed(EVENTHUB_ID, LED_CAPSL, true /*initially on*/); - mFakeEventHub->addLed(EVENTHUB_ID, LED_NUML, false /*initially off*/); - mFakeEventHub->addLed(EVENTHUB_ID, LED_SCROLLL, false /*initially off*/); - mFakeEventHub->addKey(EVENTHUB_ID, KEY_CAPSLOCK, 0, AKEYCODE_CAPS_LOCK, 0); - mFakeEventHub->addKey(EVENTHUB_ID, KEY_NUMLOCK, 0, AKEYCODE_NUM_LOCK, 0); - mFakeEventHub->addKey(EVENTHUB_ID, KEY_SCROLLLOCK, 0, AKEYCODE_SCROLL_LOCK, 0); - - KeyboardInputMapper& mapper = - constructAndAddMapper(AINPUT_SOURCE_KEYBOARD); // Initial metastate is AMETA_NONE. - ASSERT_EQ(AMETA_NONE, mapper.getMetaState()); + ASSERT_EQ(AMETA_NONE, mMapper->getMetaState()); // Initialization should have turned all of the lights off. - ASSERT_FALSE(mFakeEventHub->getLedState(EVENTHUB_ID, LED_CAPSL)); - ASSERT_FALSE(mFakeEventHub->getLedState(EVENTHUB_ID, LED_NUML)); - ASSERT_FALSE(mFakeEventHub->getLedState(EVENTHUB_ID, LED_SCROLLL)); + ASSERT_FALSE(capsLockLed); + ASSERT_FALSE(numLockLed); + ASSERT_FALSE(scrollLockLed); // Toggle caps lock on. - process(mapper, ARBITRARY_TIME, READ_TIME, EV_KEY, KEY_CAPSLOCK, 1); - process(mapper, ARBITRARY_TIME, READ_TIME, EV_KEY, KEY_CAPSLOCK, 0); - ASSERT_TRUE(mFakeEventHub->getLedState(EVENTHUB_ID, LED_CAPSL)); - ASSERT_FALSE(mFakeEventHub->getLedState(EVENTHUB_ID, LED_NUML)); - ASSERT_FALSE(mFakeEventHub->getLedState(EVENTHUB_ID, LED_SCROLLL)); - ASSERT_EQ(AMETA_CAPS_LOCK_ON, mapper.getMetaState()); + std::list argsList = process(ARBITRARY_TIME, EV_KEY, KEY_CAPSLOCK, 1); + argsList = process(ARBITRARY_TIME, EV_KEY, KEY_CAPSLOCK, 0); + ASSERT_TRUE(capsLockLed); + ASSERT_FALSE(numLockLed); + ASSERT_FALSE(scrollLockLed); + ASSERT_EQ(AMETA_CAPS_LOCK_ON, mMapper->getMetaState()); // Toggle num lock on. - process(mapper, ARBITRARY_TIME, READ_TIME, EV_KEY, KEY_NUMLOCK, 1); - process(mapper, ARBITRARY_TIME, READ_TIME, EV_KEY, KEY_NUMLOCK, 0); - ASSERT_TRUE(mFakeEventHub->getLedState(EVENTHUB_ID, LED_CAPSL)); - ASSERT_TRUE(mFakeEventHub->getLedState(EVENTHUB_ID, LED_NUML)); - ASSERT_FALSE(mFakeEventHub->getLedState(EVENTHUB_ID, LED_SCROLLL)); - ASSERT_EQ(AMETA_CAPS_LOCK_ON | AMETA_NUM_LOCK_ON, mapper.getMetaState()); + argsList = process(ARBITRARY_TIME, EV_KEY, KEY_NUMLOCK, 1); + argsList = process(ARBITRARY_TIME, EV_KEY, KEY_NUMLOCK, 0); + ASSERT_TRUE(capsLockLed); + ASSERT_TRUE(numLockLed); + ASSERT_FALSE(scrollLockLed); + ASSERT_EQ(AMETA_CAPS_LOCK_ON | AMETA_NUM_LOCK_ON, mMapper->getMetaState()); // Toggle caps lock off. - process(mapper, ARBITRARY_TIME, READ_TIME, EV_KEY, KEY_CAPSLOCK, 1); - process(mapper, ARBITRARY_TIME, READ_TIME, EV_KEY, KEY_CAPSLOCK, 0); - ASSERT_FALSE(mFakeEventHub->getLedState(EVENTHUB_ID, LED_CAPSL)); - ASSERT_TRUE(mFakeEventHub->getLedState(EVENTHUB_ID, LED_NUML)); - ASSERT_FALSE(mFakeEventHub->getLedState(EVENTHUB_ID, LED_SCROLLL)); - ASSERT_EQ(AMETA_NUM_LOCK_ON, mapper.getMetaState()); + argsList = process(ARBITRARY_TIME, EV_KEY, KEY_CAPSLOCK, 1); + argsList = process(ARBITRARY_TIME, EV_KEY, KEY_CAPSLOCK, 0); + ASSERT_FALSE(capsLockLed); + ASSERT_TRUE(numLockLed); + ASSERT_FALSE(scrollLockLed); + ASSERT_EQ(AMETA_NUM_LOCK_ON, mMapper->getMetaState()); // Toggle scroll lock on. - process(mapper, ARBITRARY_TIME, READ_TIME, EV_KEY, KEY_SCROLLLOCK, 1); - process(mapper, ARBITRARY_TIME, READ_TIME, EV_KEY, KEY_SCROLLLOCK, 0); - ASSERT_FALSE(mFakeEventHub->getLedState(EVENTHUB_ID, LED_CAPSL)); - ASSERT_TRUE(mFakeEventHub->getLedState(EVENTHUB_ID, LED_NUML)); - ASSERT_TRUE(mFakeEventHub->getLedState(EVENTHUB_ID, LED_SCROLLL)); - ASSERT_EQ(AMETA_NUM_LOCK_ON | AMETA_SCROLL_LOCK_ON, mapper.getMetaState()); + argsList = process(ARBITRARY_TIME, EV_KEY, KEY_SCROLLLOCK, 1); + argsList = process(ARBITRARY_TIME, EV_KEY, KEY_SCROLLLOCK, 0); + ASSERT_FALSE(capsLockLed); + ASSERT_TRUE(numLockLed); + ASSERT_TRUE(scrollLockLed); + ASSERT_EQ(AMETA_NUM_LOCK_ON | AMETA_SCROLL_LOCK_ON, mMapper->getMetaState()); // Toggle num lock off. - process(mapper, ARBITRARY_TIME, READ_TIME, EV_KEY, KEY_NUMLOCK, 1); - process(mapper, ARBITRARY_TIME, READ_TIME, EV_KEY, KEY_NUMLOCK, 0); - ASSERT_FALSE(mFakeEventHub->getLedState(EVENTHUB_ID, LED_CAPSL)); - ASSERT_FALSE(mFakeEventHub->getLedState(EVENTHUB_ID, LED_NUML)); - ASSERT_TRUE(mFakeEventHub->getLedState(EVENTHUB_ID, LED_SCROLLL)); - ASSERT_EQ(AMETA_SCROLL_LOCK_ON, mapper.getMetaState()); + argsList = process(ARBITRARY_TIME, EV_KEY, KEY_NUMLOCK, 1); + argsList = process(ARBITRARY_TIME, EV_KEY, KEY_NUMLOCK, 0); + ASSERT_FALSE(capsLockLed); + ASSERT_FALSE(numLockLed); + ASSERT_TRUE(scrollLockLed); + ASSERT_EQ(AMETA_SCROLL_LOCK_ON, mMapper->getMetaState()); // Toggle scroll lock off. - process(mapper, ARBITRARY_TIME, READ_TIME, EV_KEY, KEY_SCROLLLOCK, 1); - process(mapper, ARBITRARY_TIME, READ_TIME, EV_KEY, KEY_SCROLLLOCK, 0); - ASSERT_FALSE(mFakeEventHub->getLedState(EVENTHUB_ID, LED_CAPSL)); - ASSERT_FALSE(mFakeEventHub->getLedState(EVENTHUB_ID, LED_NUML)); - ASSERT_FALSE(mFakeEventHub->getLedState(EVENTHUB_ID, LED_SCROLLL)); - ASSERT_EQ(AMETA_NONE, mapper.getMetaState()); + argsList = process(ARBITRARY_TIME, EV_KEY, KEY_SCROLLLOCK, 1); + argsList = process(ARBITRARY_TIME, EV_KEY, KEY_SCROLLLOCK, 0); + ASSERT_FALSE(capsLockLed); + ASSERT_FALSE(numLockLed); + ASSERT_FALSE(scrollLockLed); + ASSERT_EQ(AMETA_NONE, mMapper->getMetaState()); +} + +TEST_F(KeyboardInputMapperUnitTest, DisablingDeviceResetsPressedKeys) { + const int32_t USAGE_A = 0x070004; + addKeyByEvdevCode(KEY_HOME, AKEYCODE_HOME, POLICY_FLAG_WAKE); + addKeyByUsageCode(USAGE_A, AKEYCODE_A, POLICY_FLAG_WAKE); + + // Key down by scan code. + std::list argsList = process(ARBITRARY_TIME, EV_KEY, KEY_HOME, 1); + NotifyKeyArgs args = expectSingleKeyArg(argsList); + ASSERT_EQ(DEVICE_ID, args.deviceId); + ASSERT_EQ(AINPUT_SOURCE_KEYBOARD, args.source); + ASSERT_EQ(ARBITRARY_TIME, args.eventTime); + ASSERT_EQ(AKEY_EVENT_ACTION_DOWN, args.action); + ASSERT_EQ(AKEYCODE_HOME, args.keyCode); + ASSERT_EQ(KEY_HOME, args.scanCode); + ASSERT_EQ(AKEY_EVENT_FLAG_FROM_SYSTEM, args.flags); + + // Disable device, it should synthesize cancellation events for down events. + mReaderConfiguration.disabledDevices.insert(DEVICE_ID); + argsList = mMapper->reconfigure(ARBITRARY_TIME, mReaderConfiguration, + InputReaderConfiguration::Change::ENABLED_STATE); + argsList += mMapper->reset(ARBITRARY_TIME); + args = expectSingleKeyArg(argsList); + ASSERT_EQ(AKEY_EVENT_ACTION_UP, args.action); + ASSERT_EQ(AKEYCODE_HOME, args.keyCode); + ASSERT_EQ(KEY_HOME, args.scanCode); + ASSERT_EQ(AKEY_EVENT_FLAG_FROM_SYSTEM | AKEY_EVENT_FLAG_CANCELED, args.flags); +} + +TEST_F(KeyboardInputMapperUnitTest, Configure_AssignKeyboardLayoutInfo) { + std::list unused = + mMapper->reconfigure(ARBITRARY_TIME, mReaderConfiguration, /*changes=*/{}); + + int32_t generation = mDevice->getGeneration(); + mReaderConfiguration.keyboardLayoutAssociations.insert( + {mIdentifier.location, DEVICE_KEYBOARD_LAYOUT_INFO}); + + unused += mMapper->reconfigure(ARBITRARY_TIME, mReaderConfiguration, + InputReaderConfiguration::Change::KEYBOARD_LAYOUT_ASSOCIATION); + + InputDeviceInfo deviceInfo; + mMapper->populateDeviceInfo(deviceInfo); + ASSERT_EQ(DEVICE_KEYBOARD_LAYOUT_INFO.languageTag, + deviceInfo.getKeyboardLayoutInfo()->languageTag); + ASSERT_EQ(DEVICE_KEYBOARD_LAYOUT_INFO.layoutType, + deviceInfo.getKeyboardLayoutInfo()->layoutType); + ASSERT_GT(mDevice->getGeneration(), generation); + + // Call change layout association with the same values: Generation shouldn't change + generation = mDevice->getGeneration(); + unused += mMapper->reconfigure(ARBITRARY_TIME, mReaderConfiguration, + InputReaderConfiguration::Change::KEYBOARD_LAYOUT_ASSOCIATION); + ASSERT_EQ(mDevice->getGeneration(), generation); +} + +TEST_F(KeyboardInputMapperUnitTest, LayoutInfoCorrectlyMapped) { + EXPECT_CALL(mMockEventHub, getRawLayoutInfo(EVENTHUB_ID)) + .WillRepeatedly(Return(RawLayoutInfo{.languageTag = "en", .layoutType = "extended"})); + + // Configuration + std::list unused = + mMapper->reconfigure(ARBITRARY_TIME, mReaderConfiguration, /*changes=*/{}); + + InputDeviceInfo deviceInfo; + mMapper->populateDeviceInfo(deviceInfo); + ASSERT_EQ("en", deviceInfo.getKeyboardLayoutInfo()->languageTag); + ASSERT_EQ("extended", deviceInfo.getKeyboardLayoutInfo()->layoutType); +} + +TEST_F(KeyboardInputMapperUnitTest, Process_GestureEventToSetFlagKeepTouchMode) { + addKeyByEvdevCode(KEY_LEFT, AKEYCODE_DPAD_LEFT, POLICY_FLAG_GESTURE); + + // Key down + std::list argsList = process(ARBITRARY_TIME, EV_KEY, KEY_LEFT, 1); + ASSERT_EQ(AKEY_EVENT_FLAG_FROM_SYSTEM | AKEY_EVENT_FLAG_KEEP_TOUCH_MODE, + expectSingleKeyArg(argsList).flags); +} + +TEST_F_WITH_FLAGS(KeyboardInputMapperUnitTest, WakeBehavior_AlphabeticKeyboard, + REQUIRES_FLAGS_ENABLED(ACONFIG_FLAG(com::android::input::flags, + enable_alphabetic_keyboard_wake))) { + // For internal alphabetic devices, keys will trigger wake on key down. + + addKeyByEvdevCode(KEY_A, AKEYCODE_A); + addKeyByEvdevCode(KEY_HOME, AKEYCODE_HOME); + addKeyByEvdevCode(KEY_PLAYPAUSE, AKEYCODE_MEDIA_PLAY_PAUSE); + + std::list argsList = process(ARBITRARY_TIME, EV_KEY, KEY_A, 1); + ASSERT_EQ(POLICY_FLAG_WAKE, expectSingleKeyArg(argsList).policyFlags); + + argsList = process(ARBITRARY_TIME + 1, EV_KEY, KEY_A, 0); + ASSERT_EQ(uint32_t(0), expectSingleKeyArg(argsList).policyFlags); + + argsList = process(ARBITRARY_TIME, EV_KEY, KEY_HOME, 1); + ASSERT_EQ(POLICY_FLAG_WAKE, expectSingleKeyArg(argsList).policyFlags); + + argsList = process(ARBITRARY_TIME + 1, EV_KEY, KEY_HOME, 0); + ASSERT_EQ(uint32_t(0), expectSingleKeyArg(argsList).policyFlags); + + argsList = process(ARBITRARY_TIME, EV_KEY, KEY_PLAYPAUSE, 1); + ASSERT_EQ(POLICY_FLAG_WAKE, expectSingleKeyArg(argsList).policyFlags); + + argsList = process(ARBITRARY_TIME + 1, EV_KEY, KEY_PLAYPAUSE, 0); + ASSERT_EQ(uint32_t(0), expectSingleKeyArg(argsList).policyFlags); +} + +// --- KeyboardInputMapperTest --- + +// TODO(b/283812079): convert the tests for this class, which use multiple mappers each, to use +// InputMapperUnitTest. +class KeyboardInputMapperTest : public InputMapperTest { +protected: + void SetUp() override { + InputMapperTest::SetUp(DEVICE_CLASSES | InputDeviceClass::KEYBOARD | + InputDeviceClass::ALPHAKEY); + } + const std::string UNIQUE_ID = "local:0"; + + void testDPadKeyRotation(KeyboardInputMapper& mapper, int32_t originalScanCode, + int32_t originalKeyCode, int32_t rotatedKeyCode, + ui::LogicalDisplayId displayId = ui::LogicalDisplayId::INVALID); +}; + +void KeyboardInputMapperTest::testDPadKeyRotation(KeyboardInputMapper& mapper, + int32_t originalScanCode, int32_t originalKeyCode, + int32_t rotatedKeyCode, + ui::LogicalDisplayId displayId) { + NotifyKeyArgs args; + + process(mapper, ARBITRARY_TIME, READ_TIME, EV_KEY, originalScanCode, 1); + ASSERT_NO_FATAL_FAILURE(mFakeListener->assertNotifyKeyWasCalled(&args)); + ASSERT_EQ(AKEY_EVENT_ACTION_DOWN, args.action); + ASSERT_EQ(originalScanCode, args.scanCode); + ASSERT_EQ(rotatedKeyCode, args.keyCode); + ASSERT_EQ(displayId, args.displayId); + + process(mapper, ARBITRARY_TIME, READ_TIME, EV_KEY, originalScanCode, 0); + ASSERT_NO_FATAL_FAILURE(mFakeListener->assertNotifyKeyWasCalled(&args)); + ASSERT_EQ(AKEY_EVENT_ACTION_UP, args.action); + ASSERT_EQ(originalScanCode, args.scanCode); + ASSERT_EQ(rotatedKeyCode, args.keyCode); + ASSERT_EQ(displayId, args.displayId); } TEST_F(KeyboardInputMapperTest, Configure_AssignsDisplayPort) { @@ -893,126 +1008,6 @@ TEST_F(KeyboardInputMapperTest, Process_LockedKeysShouldToggleInMultiDevices) { ASSERT_EQ(AMETA_NONE, mapper2.getMetaState()); } -TEST_F(KeyboardInputMapperTest, Process_DisabledDevice) { - const int32_t USAGE_A = 0x070004; - mFakeEventHub->addKey(EVENTHUB_ID, KEY_HOME, 0, AKEYCODE_HOME, POLICY_FLAG_WAKE); - mFakeEventHub->addKey(EVENTHUB_ID, 0, USAGE_A, AKEYCODE_A, POLICY_FLAG_WAKE); - - KeyboardInputMapper& mapper = - constructAndAddMapper(AINPUT_SOURCE_KEYBOARD); - // Key down by scan code. - process(mapper, ARBITRARY_TIME, READ_TIME, EV_KEY, KEY_HOME, 1); - NotifyKeyArgs args; - ASSERT_NO_FATAL_FAILURE(mFakeListener->assertNotifyKeyWasCalled(&args)); - ASSERT_EQ(DEVICE_ID, args.deviceId); - ASSERT_EQ(AINPUT_SOURCE_KEYBOARD, args.source); - ASSERT_EQ(ARBITRARY_TIME, args.eventTime); - ASSERT_EQ(AKEY_EVENT_ACTION_DOWN, args.action); - ASSERT_EQ(AKEYCODE_HOME, args.keyCode); - ASSERT_EQ(KEY_HOME, args.scanCode); - ASSERT_EQ(AKEY_EVENT_FLAG_FROM_SYSTEM, args.flags); - - // Disable device, it should synthesize cancellation events for down events. - mFakePolicy->addDisabledDevice(DEVICE_ID); - configureDevice(InputReaderConfiguration::Change::ENABLED_STATE); - - ASSERT_NO_FATAL_FAILURE(mFakeListener->assertNotifyKeyWasCalled(&args)); - ASSERT_EQ(AKEY_EVENT_ACTION_UP, args.action); - ASSERT_EQ(AKEYCODE_HOME, args.keyCode); - ASSERT_EQ(KEY_HOME, args.scanCode); - ASSERT_EQ(AKEY_EVENT_FLAG_FROM_SYSTEM | AKEY_EVENT_FLAG_CANCELED, args.flags); -} - -TEST_F(KeyboardInputMapperTest, Configure_AssignKeyboardLayoutInfo) { - constructAndAddMapper(AINPUT_SOURCE_KEYBOARD); - std::list unused = - mDevice->configure(ARBITRARY_TIME, mFakePolicy->getReaderConfiguration(), - /*changes=*/{}); - - uint32_t generation = mReader->getContext()->getGeneration(); - mFakePolicy->addKeyboardLayoutAssociation(DEVICE_LOCATION, DEVICE_KEYBOARD_LAYOUT_INFO); - - unused += mDevice->configure(ARBITRARY_TIME, mFakePolicy->getReaderConfiguration(), - InputReaderConfiguration::Change::KEYBOARD_LAYOUT_ASSOCIATION); - - InputDeviceInfo deviceInfo = mDevice->getDeviceInfo(); - ASSERT_EQ(DEVICE_KEYBOARD_LAYOUT_INFO.languageTag, - deviceInfo.getKeyboardLayoutInfo()->languageTag); - ASSERT_EQ(DEVICE_KEYBOARD_LAYOUT_INFO.layoutType, - deviceInfo.getKeyboardLayoutInfo()->layoutType); - ASSERT_TRUE(mReader->getContext()->getGeneration() != generation); - - // Call change layout association with the same values: Generation shouldn't change - generation = mReader->getContext()->getGeneration(); - mFakePolicy->addKeyboardLayoutAssociation(DEVICE_LOCATION, DEVICE_KEYBOARD_LAYOUT_INFO); - unused += mDevice->configure(ARBITRARY_TIME, mFakePolicy->getReaderConfiguration(), - InputReaderConfiguration::Change::KEYBOARD_LAYOUT_ASSOCIATION); - ASSERT_TRUE(mReader->getContext()->getGeneration() == generation); -} - -TEST_F(KeyboardInputMapperTest, LayoutInfoCorrectlyMapped) { - mFakeEventHub->setRawLayoutInfo(EVENTHUB_ID, - RawLayoutInfo{.languageTag = "en", .layoutType = "extended"}); - - // Configuration - constructAndAddMapper(AINPUT_SOURCE_KEYBOARD); - InputReaderConfiguration config; - std::list unused = mDevice->configure(ARBITRARY_TIME, config, /*changes=*/{}); - - ASSERT_EQ("en", mDevice->getDeviceInfo().getKeyboardLayoutInfo()->languageTag); - ASSERT_EQ("extended", mDevice->getDeviceInfo().getKeyboardLayoutInfo()->layoutType); -} - -TEST_F(KeyboardInputMapperTest, Process_GesureEventToSetFlagKeepTouchMode) { - mFakeEventHub->addKey(EVENTHUB_ID, KEY_LEFT, 0, AKEYCODE_DPAD_LEFT, POLICY_FLAG_GESTURE); - KeyboardInputMapper& mapper = - constructAndAddMapper(AINPUT_SOURCE_KEYBOARD); - NotifyKeyArgs args; - - // Key down - process(mapper, ARBITRARY_TIME, READ_TIME, EV_KEY, KEY_LEFT, 1); - ASSERT_NO_FATAL_FAILURE(mFakeListener->assertNotifyKeyWasCalled(&args)); - ASSERT_EQ(AKEY_EVENT_FLAG_FROM_SYSTEM | AKEY_EVENT_FLAG_KEEP_TOUCH_MODE, args.flags); -} - -TEST_F_WITH_FLAGS(KeyboardInputMapperTest, WakeBehavior_AlphabeticKeyboard, - REQUIRES_FLAGS_ENABLED(ACONFIG_FLAG(com::android::input::flags, - enable_alphabetic_keyboard_wake))) { - // For internal alphabetic devices, keys will trigger wake on key down. - - mFakeEventHub->addKey(EVENTHUB_ID, KEY_A, 0, AKEYCODE_A, 0); - mFakeEventHub->addKey(EVENTHUB_ID, KEY_HOME, 0, AKEYCODE_HOME, 0); - mFakeEventHub->addKey(EVENTHUB_ID, KEY_PLAYPAUSE, 0, AKEYCODE_MEDIA_PLAY_PAUSE, 0); - - KeyboardInputMapper& mapper = - constructAndAddMapper(AINPUT_SOURCE_KEYBOARD); - - process(mapper, ARBITRARY_TIME, READ_TIME, EV_KEY, KEY_A, 1); - NotifyKeyArgs args; - ASSERT_NO_FATAL_FAILURE(mFakeListener->assertNotifyKeyWasCalled(&args)); - ASSERT_EQ(POLICY_FLAG_WAKE, args.policyFlags); - - process(mapper, ARBITRARY_TIME + 1, READ_TIME, EV_KEY, KEY_A, 0); - ASSERT_NO_FATAL_FAILURE(mFakeListener->assertNotifyKeyWasCalled(&args)); - ASSERT_EQ(uint32_t(0), args.policyFlags); - - process(mapper, ARBITRARY_TIME, READ_TIME, EV_KEY, KEY_HOME, 1); - ASSERT_NO_FATAL_FAILURE(mFakeListener->assertNotifyKeyWasCalled(&args)); - ASSERT_EQ(POLICY_FLAG_WAKE, args.policyFlags); - - process(mapper, ARBITRARY_TIME + 1, READ_TIME, EV_KEY, KEY_HOME, 0); - ASSERT_NO_FATAL_FAILURE(mFakeListener->assertNotifyKeyWasCalled(&args)); - ASSERT_EQ(uint32_t(0), args.policyFlags); - - process(mapper, ARBITRARY_TIME, READ_TIME, EV_KEY, KEY_PLAYPAUSE, 1); - ASSERT_NO_FATAL_FAILURE(mFakeListener->assertNotifyKeyWasCalled(&args)); - ASSERT_EQ(POLICY_FLAG_WAKE, args.policyFlags); - - process(mapper, ARBITRARY_TIME + 1, READ_TIME, EV_KEY, KEY_PLAYPAUSE, 0); - ASSERT_NO_FATAL_FAILURE(mFakeListener->assertNotifyKeyWasCalled(&args)); - ASSERT_EQ(uint32_t(0), args.policyFlags); -} - /** * When there is more than one KeyboardInputMapper for an InputDevice, each mapper should produce * events that use the shared keyboard source across all mappers. This is to ensure that each @@ -1060,21 +1055,34 @@ TEST_F(KeyboardInputMapperTest, UsesSharedKeyboardSource) { // --- KeyboardInputMapperTest_ExternalAlphabeticDevice --- -class KeyboardInputMapperTest_ExternalAlphabeticDevice : public InputMapperTest { +class KeyboardInputMapperTest_ExternalAlphabeticDevice : public KeyboardInputMapperUnitTest { protected: void SetUp() override { - InputMapperTest::SetUp(DEVICE_CLASSES | InputDeviceClass::KEYBOARD | - InputDeviceClass::ALPHAKEY | InputDeviceClass::EXTERNAL); + InputMapperUnitTest::SetUp(); + ON_CALL((*mDevice), getSources).WillByDefault(Return(AINPUT_SOURCE_KEYBOARD)); + ON_CALL((*mDevice), getKeyboardType).WillByDefault(Return(KeyboardType::ALPHABETIC)); + ON_CALL((*mDevice), isExternal).WillByDefault(Return(true)); + EXPECT_CALL(mMockEventHub, getDeviceClasses(EVENTHUB_ID)) + .WillRepeatedly(Return(InputDeviceClass::KEYBOARD | InputDeviceClass::ALPHAKEY | + InputDeviceClass::EXTERNAL)); + mMapper = createInputMapper(*mDeviceContext, mReaderConfiguration, + AINPUT_SOURCE_KEYBOARD); } }; // --- KeyboardInputMapperTest_ExternalNonAlphabeticDevice --- -class KeyboardInputMapperTest_ExternalNonAlphabeticDevice : public InputMapperTest { +class KeyboardInputMapperTest_ExternalNonAlphabeticDevice : public KeyboardInputMapperUnitTest { protected: void SetUp() override { - InputMapperTest::SetUp(DEVICE_CLASSES | InputDeviceClass::KEYBOARD | - InputDeviceClass::EXTERNAL); + InputMapperUnitTest::SetUp(); + ON_CALL((*mDevice), getSources).WillByDefault(Return(AINPUT_SOURCE_KEYBOARD)); + ON_CALL((*mDevice), getKeyboardType).WillByDefault(Return(KeyboardType::NON_ALPHABETIC)); + ON_CALL((*mDevice), isExternal).WillByDefault(Return(true)); + EXPECT_CALL(mMockEventHub, getDeviceClasses(EVENTHUB_ID)) + .WillRepeatedly(Return(InputDeviceClass::KEYBOARD | InputDeviceClass::EXTERNAL)); + mMapper = createInputMapper(*mDeviceContext, mReaderConfiguration, + AINPUT_SOURCE_KEYBOARD); } }; @@ -1082,104 +1090,77 @@ TEST_F(KeyboardInputMapperTest_ExternalAlphabeticDevice, WakeBehavior_Alphabetic // For external devices, keys will trigger wake on key down. Media keys should also trigger // wake if triggered from external devices. - mFakeEventHub->addKey(EVENTHUB_ID, KEY_HOME, 0, AKEYCODE_HOME, 0); - mFakeEventHub->addKey(EVENTHUB_ID, KEY_PLAY, 0, AKEYCODE_MEDIA_PLAY, 0); - mFakeEventHub->addKey(EVENTHUB_ID, KEY_PLAYPAUSE, 0, AKEYCODE_MEDIA_PLAY_PAUSE, - POLICY_FLAG_WAKE); + addKeyByEvdevCode(KEY_HOME, AKEYCODE_HOME); + addKeyByEvdevCode(KEY_PLAY, AKEYCODE_MEDIA_PLAY); + addKeyByEvdevCode(KEY_PLAYPAUSE, AKEYCODE_MEDIA_PLAY_PAUSE, POLICY_FLAG_WAKE); - KeyboardInputMapper& mapper = - constructAndAddMapper(AINPUT_SOURCE_KEYBOARD); + std::list argsList = process(ARBITRARY_TIME, EV_KEY, KEY_HOME, 1); + ASSERT_EQ(POLICY_FLAG_WAKE, expectSingleKeyArg(argsList).policyFlags); - process(mapper, ARBITRARY_TIME, READ_TIME, EV_KEY, KEY_HOME, 1); - NotifyKeyArgs args; - ASSERT_NO_FATAL_FAILURE(mFakeListener->assertNotifyKeyWasCalled(&args)); - ASSERT_EQ(POLICY_FLAG_WAKE, args.policyFlags); + argsList = process(ARBITRARY_TIME + 1, EV_KEY, KEY_HOME, 0); + ASSERT_EQ(uint32_t(0), expectSingleKeyArg(argsList).policyFlags); - process(mapper, ARBITRARY_TIME + 1, READ_TIME, EV_KEY, KEY_HOME, 0); - ASSERT_NO_FATAL_FAILURE(mFakeListener->assertNotifyKeyWasCalled(&args)); - ASSERT_EQ(uint32_t(0), args.policyFlags); + argsList = process(ARBITRARY_TIME, EV_KEY, KEY_PLAY, 1); + ASSERT_EQ(POLICY_FLAG_WAKE, expectSingleKeyArg(argsList).policyFlags); - process(mapper, ARBITRARY_TIME, READ_TIME, EV_KEY, KEY_PLAY, 1); - ASSERT_NO_FATAL_FAILURE(mFakeListener->assertNotifyKeyWasCalled(&args)); - ASSERT_EQ(POLICY_FLAG_WAKE, args.policyFlags); - - process(mapper, ARBITRARY_TIME + 1, READ_TIME, EV_KEY, KEY_PLAY, 0); - ASSERT_NO_FATAL_FAILURE(mFakeListener->assertNotifyKeyWasCalled(&args)); - ASSERT_EQ(uint32_t(0), args.policyFlags); + argsList = process(ARBITRARY_TIME + 1, EV_KEY, KEY_PLAY, 0); + ASSERT_EQ(uint32_t(0), expectSingleKeyArg(argsList).policyFlags); - process(mapper, ARBITRARY_TIME, READ_TIME, EV_KEY, KEY_PLAYPAUSE, 1); - ASSERT_NO_FATAL_FAILURE(mFakeListener->assertNotifyKeyWasCalled(&args)); - ASSERT_EQ(POLICY_FLAG_WAKE, args.policyFlags); + argsList = process(ARBITRARY_TIME, EV_KEY, KEY_PLAYPAUSE, 1); + ASSERT_EQ(POLICY_FLAG_WAKE, expectSingleKeyArg(argsList).policyFlags); - process(mapper, ARBITRARY_TIME + 1, READ_TIME, EV_KEY, KEY_PLAYPAUSE, 0); - ASSERT_NO_FATAL_FAILURE(mFakeListener->assertNotifyKeyWasCalled(&args)); - ASSERT_EQ(POLICY_FLAG_WAKE, args.policyFlags); + argsList = process(ARBITRARY_TIME + 1, EV_KEY, KEY_PLAYPAUSE, 0); + ASSERT_EQ(POLICY_FLAG_WAKE, expectSingleKeyArg(argsList).policyFlags); } TEST_F(KeyboardInputMapperTest_ExternalNonAlphabeticDevice, WakeBehavior_NonAlphabeticKeyboard) { // For external devices, keys will trigger wake on key down. Media keys should not trigger // wake if triggered from external non-alphaebtic keyboard (e.g. headsets). - mFakeEventHub->addKey(EVENTHUB_ID, KEY_PLAY, 0, AKEYCODE_MEDIA_PLAY, 0); - mFakeEventHub->addKey(EVENTHUB_ID, KEY_PLAYPAUSE, 0, AKEYCODE_MEDIA_PLAY_PAUSE, - POLICY_FLAG_WAKE); + addKeyByEvdevCode(KEY_PLAY, AKEYCODE_MEDIA_PLAY); + addKeyByEvdevCode(KEY_PLAYPAUSE, AKEYCODE_MEDIA_PLAY_PAUSE, POLICY_FLAG_WAKE); - KeyboardInputMapper& mapper = - constructAndAddMapper(AINPUT_SOURCE_KEYBOARD); - - process(mapper, ARBITRARY_TIME, READ_TIME, EV_KEY, KEY_PLAY, 1); - NotifyKeyArgs args; - ASSERT_NO_FATAL_FAILURE(mFakeListener->assertNotifyKeyWasCalled(&args)); - ASSERT_EQ(uint32_t(0), args.policyFlags); + std::list argsList = process(ARBITRARY_TIME, EV_KEY, KEY_PLAY, 1); + ASSERT_EQ(uint32_t(0), expectSingleKeyArg(argsList).policyFlags); - process(mapper, ARBITRARY_TIME + 1, READ_TIME, EV_KEY, KEY_PLAY, 0); - ASSERT_NO_FATAL_FAILURE(mFakeListener->assertNotifyKeyWasCalled(&args)); - ASSERT_EQ(uint32_t(0), args.policyFlags); + argsList = process(ARBITRARY_TIME + 1, EV_KEY, KEY_PLAY, 0); + ASSERT_EQ(uint32_t(0), expectSingleKeyArg(argsList).policyFlags); - process(mapper, ARBITRARY_TIME, READ_TIME, EV_KEY, KEY_PLAYPAUSE, 1); - ASSERT_NO_FATAL_FAILURE(mFakeListener->assertNotifyKeyWasCalled(&args)); - ASSERT_EQ(POLICY_FLAG_WAKE, args.policyFlags); + argsList = process(ARBITRARY_TIME, EV_KEY, KEY_PLAYPAUSE, 1); + ASSERT_EQ(POLICY_FLAG_WAKE, expectSingleKeyArg(argsList).policyFlags); - process(mapper, ARBITRARY_TIME + 1, READ_TIME, EV_KEY, KEY_PLAYPAUSE, 0); - ASSERT_NO_FATAL_FAILURE(mFakeListener->assertNotifyKeyWasCalled(&args)); - ASSERT_EQ(POLICY_FLAG_WAKE, args.policyFlags); + argsList = process(ARBITRARY_TIME + 1, EV_KEY, KEY_PLAYPAUSE, 0); + ASSERT_EQ(POLICY_FLAG_WAKE, expectSingleKeyArg(argsList).policyFlags); } TEST_F(KeyboardInputMapperTest_ExternalAlphabeticDevice, DoNotWakeByDefaultBehavior) { // Tv Remote key's wake behavior is prescribed by the keylayout file. - mFakeEventHub->addKey(EVENTHUB_ID, KEY_HOME, 0, AKEYCODE_HOME, POLICY_FLAG_WAKE); - mFakeEventHub->addKey(EVENTHUB_ID, KEY_DOWN, 0, AKEYCODE_DPAD_DOWN, 0); - mFakeEventHub->addKey(EVENTHUB_ID, KEY_PLAY, 0, AKEYCODE_MEDIA_PLAY, POLICY_FLAG_WAKE); + addKeyByEvdevCode(KEY_HOME, AKEYCODE_HOME, POLICY_FLAG_WAKE); + addKeyByEvdevCode(KEY_DOWN, AKEYCODE_DPAD_DOWN); + addKeyByEvdevCode(KEY_PLAY, AKEYCODE_MEDIA_PLAY, POLICY_FLAG_WAKE); - addConfigurationProperty("keyboard.doNotWakeByDefault", "1"); - KeyboardInputMapper& mapper = - constructAndAddMapper(AINPUT_SOURCE_KEYBOARD); + mPropertyMap.addProperty("keyboard.doNotWakeByDefault", "1"); + mMapper = createInputMapper(*mDeviceContext, mReaderConfiguration, + AINPUT_SOURCE_KEYBOARD); - process(mapper, ARBITRARY_TIME, READ_TIME, EV_KEY, KEY_HOME, 1); - NotifyKeyArgs args; - ASSERT_NO_FATAL_FAILURE(mFakeListener->assertNotifyKeyWasCalled(&args)); - ASSERT_EQ(POLICY_FLAG_WAKE, args.policyFlags); + std::list argsList = process(ARBITRARY_TIME, EV_KEY, KEY_HOME, 1); + ASSERT_EQ(POLICY_FLAG_WAKE, expectSingleKeyArg(argsList).policyFlags); - process(mapper, ARBITRARY_TIME + 1, READ_TIME, EV_KEY, KEY_HOME, 0); - ASSERT_NO_FATAL_FAILURE(mFakeListener->assertNotifyKeyWasCalled(&args)); - ASSERT_EQ(POLICY_FLAG_WAKE, args.policyFlags); + argsList = process(ARBITRARY_TIME + 1, EV_KEY, KEY_HOME, 0); + ASSERT_EQ(POLICY_FLAG_WAKE, expectSingleKeyArg(argsList).policyFlags); - process(mapper, ARBITRARY_TIME, READ_TIME, EV_KEY, KEY_DOWN, 1); - ASSERT_NO_FATAL_FAILURE(mFakeListener->assertNotifyKeyWasCalled(&args)); - ASSERT_EQ(uint32_t(0), args.policyFlags); + argsList = process(ARBITRARY_TIME, EV_KEY, KEY_DOWN, 1); + ASSERT_EQ(uint32_t(0), expectSingleKeyArg(argsList).policyFlags); - process(mapper, ARBITRARY_TIME + 1, READ_TIME, EV_KEY, KEY_DOWN, 0); - ASSERT_NO_FATAL_FAILURE(mFakeListener->assertNotifyKeyWasCalled(&args)); - ASSERT_EQ(uint32_t(0), args.policyFlags); + argsList = process(ARBITRARY_TIME + 1, EV_KEY, KEY_DOWN, 0); + ASSERT_EQ(uint32_t(0), expectSingleKeyArg(argsList).policyFlags); - process(mapper, ARBITRARY_TIME, READ_TIME, EV_KEY, KEY_PLAY, 1); - ASSERT_NO_FATAL_FAILURE(mFakeListener->assertNotifyKeyWasCalled(&args)); - ASSERT_EQ(POLICY_FLAG_WAKE, args.policyFlags); + argsList = process(ARBITRARY_TIME, EV_KEY, KEY_PLAY, 1); + ASSERT_EQ(POLICY_FLAG_WAKE, expectSingleKeyArg(argsList).policyFlags); - process(mapper, ARBITRARY_TIME + 1, READ_TIME, EV_KEY, KEY_PLAY, 0); - ASSERT_NO_FATAL_FAILURE(mFakeListener->assertNotifyKeyWasCalled(&args)); - ASSERT_EQ(POLICY_FLAG_WAKE, args.policyFlags); + argsList = process(ARBITRARY_TIME + 1, EV_KEY, KEY_PLAY, 0); + ASSERT_EQ(POLICY_FLAG_WAKE, expectSingleKeyArg(argsList).policyFlags); } } // namespace android -- cgit v1.2.3-59-g8ed1b From 2358c1360dee84841d9d405cb0de5fa199fd0cf1 Mon Sep 17 00:00:00 2001 From: Harry Cutts Date: Tue, 19 Nov 2024 18:34:59 +0000 Subject: MotionEvent: add safe dumping method This method should allow us to log more details of an event in case of validation failures, without potential for infinite recursion in getter methods. Bug: 379368465 Test: add a test to MotionEventTest that calls getHistoricalRawPointerCoords with an invalid pointer index, and check the logs when run with `atest --host` Flag: EXEMPT logs only Change-Id: I9c7084cedbc7e6f6834cd1b401da04d07d22ce35 --- include/input/Input.h | 9 ++++++++ libs/input/Input.cpp | 63 ++++++++++++++++++++++++++++++++++++++++++++------- 2 files changed, 64 insertions(+), 8 deletions(-) (limited to 'include/input') diff --git a/include/input/Input.h b/include/input/Input.h index a8684bd19b..0e330e453b 100644 --- a/include/input/Input.h +++ b/include/input/Input.h @@ -994,6 +994,15 @@ protected: std::vector mPointerProperties; std::vector mSampleEventTimes; std::vector mSamplePointerCoords; + +private: + /** + * Create a human-readable string representation of the event's data for debugging purposes. + * + * Unlike operator<<, this method does not assume that the event data is valid or consistent, or + * call any accessor methods that might themselves call safeDump in the case of invalid data. + */ + std::string safeDump() const; }; std::ostream& operator<<(std::ostream& out, const MotionEvent& event); diff --git a/libs/input/Input.cpp b/libs/input/Input.cpp index b87a7068b5..65a088eb6d 100644 --- a/libs/input/Input.cpp +++ b/libs/input/Input.cpp @@ -685,11 +685,12 @@ void MotionEvent::setCursorPosition(float x, float y) { const PointerCoords* MotionEvent::getRawPointerCoords(size_t pointerIndex) const { if (CC_UNLIKELY(pointerIndex < 0 || pointerIndex >= getPointerCount())) { - LOG(FATAL) << __func__ << ": Invalid pointer index " << pointerIndex << " for " << *this; + LOG(FATAL) << __func__ << ": Invalid pointer index " << pointerIndex << " for " + << safeDump(); } const size_t position = getHistorySize() * getPointerCount() + pointerIndex; if (CC_UNLIKELY(position < 0 || position >= mSamplePointerCoords.size())) { - LOG(FATAL) << __func__ << ": Invalid array index " << position << " for " << *this; + LOG(FATAL) << __func__ << ": Invalid array index " << position << " for " << safeDump(); } return &mSamplePointerCoords[position]; } @@ -705,17 +706,16 @@ float MotionEvent::getAxisValue(int32_t axis, size_t pointerIndex) const { const PointerCoords* MotionEvent::getHistoricalRawPointerCoords( size_t pointerIndex, size_t historicalIndex) const { if (CC_UNLIKELY(pointerIndex < 0 || pointerIndex >= getPointerCount())) { - LOG(FATAL) << __func__ << ": Invalid pointer index " << pointerIndex - << "; should be between >0 and ≤" << getPointerCount(); + LOG(FATAL) << __func__ << ": Invalid pointer index " << pointerIndex << " for " + << safeDump(); } if (CC_UNLIKELY(historicalIndex < 0 || historicalIndex > getHistorySize())) { - LOG(FATAL) << __func__ << ": Invalid historical index " << historicalIndex - << "; should be >0 and ≤" << getHistorySize(); + LOG(FATAL) << __func__ << ": Invalid historical index " << historicalIndex << " for " + << safeDump(); } const size_t position = historicalIndex * getPointerCount() + pointerIndex; if (CC_UNLIKELY(position < 0 || position >= mSamplePointerCoords.size())) { - LOG(FATAL) << __func__ << ": Invalid array index " << position << "; should be >0 and ≤" - << mSamplePointerCoords.size(); + LOG(FATAL) << __func__ << ": Invalid array index " << position << " for " << safeDump(); } return &mSamplePointerCoords[position]; } @@ -1146,6 +1146,53 @@ bool MotionEvent::operator==(const android::MotionEvent& o) const { // clang-format on } +std::string MotionEvent::safeDump() const { + std::stringstream out; + // Field names have the m prefix here to make it easy to distinguish safeDump output from + // operator<< output in logs. + out << "MotionEvent { mAction=" << MotionEvent::actionToString(mAction); + if (mActionButton != 0) { + out << ", mActionButton=" << mActionButton; + } + if (mButtonState != 0) { + out << ", mButtonState=" << mButtonState; + } + if (mClassification != MotionClassification::NONE) { + out << ", mClassification=" << motionClassificationToString(mClassification); + } + if (mMetaState != 0) { + out << ", mMetaState=" << mMetaState; + } + if (mFlags != 0) { + out << ", mFlags=0x" << std::hex << mFlags << std::dec; + } + if (mEdgeFlags != 0) { + out << ", mEdgeFlags=" << mEdgeFlags; + } + out << ", mDownTime=" << mDownTime; + out << ", mDeviceId=" << mDeviceId; + out << ", mSource=" << inputEventSourceToString(mSource); + out << ", mDisplayId=" << mDisplayId; + out << ", mEventId=0x" << std::hex << mId << std::dec; + // Since we're not assuming the data is at all valid, we also limit the number of items that + // might be printed from vectors, in case the vector's size field is corrupted. + out << ", mPointerProperties=(" << mPointerProperties.size() << ")["; + for (size_t i = 0; i < mPointerProperties.size() && i < MAX_POINTERS; i++) { + out << (i > 0 ? ", " : "") << mPointerProperties.at(i); + } + out << "], mSampleEventTimes=(" << mSampleEventTimes.size() << ")["; + for (size_t i = 0; i < mSampleEventTimes.size() && i < 256; i++) { + out << (i > 0 ? ", " : "") << mSampleEventTimes.at(i); + } + out << "], mSamplePointerCoords=(" << mSamplePointerCoords.size() << ")["; + for (size_t i = 0; i < mSamplePointerCoords.size() && i < MAX_POINTERS; i++) { + const PointerCoords& coords = mSamplePointerCoords.at(i); + out << (i > 0 ? ", " : "") << "(" << coords.getX() << ", " << coords.getY() << ")"; + } + out << "] }"; + return out.str(); +} + std::ostream& operator<<(std::ostream& out, const MotionEvent& event) { out << "MotionEvent { action=" << MotionEvent::actionToString(event.getAction()); if (event.getActionButton() != 0) { -- cgit v1.2.3-59-g8ed1b From 53463397c3b84bb71d7fe155a8ada86fdcc0e96b Mon Sep 17 00:00:00 2001 From: Paul Ramirez Date: Mon, 11 Nov 2024 21:49:51 +0000 Subject: Fix One Euro filter's units of computation Changed the units that the One Euro filter uses to compute the filtered coordinates. This was causing a crash because if two timestamps were sufficiently close to each other, by the time of implicitly converting from nanoseconds to seconds, they were considered equal. This led to a zero division when calculating the sampling frequency. Now, everything is handled in the scale of nanoseconds, and conversion are done if and only if they're necessary. Bug: 297226446 Flag: EXEMPT bugfix Test: TEST=libinput_tests; m $TEST && $ANDROID_HOST_OUT/nativetest64/$TEST/$TEST Change-Id: I7fced6db447074cccb3d938eb9dc7a9707433f53 --- include/input/CoordinateFilter.h | 2 +- include/input/OneEuroFilter.h | 10 +- libs/input/CoordinateFilter.cpp | 2 +- libs/input/OneEuroFilter.cpp | 34 ++-- libs/input/tests/Android.bp | 1 + .../tests/InputConsumerFilteredResampling_test.cpp | 218 +++++++++++++++++++++ libs/input/tests/OneEuroFilter_test.cpp | 5 +- libs/input/tests/TestEventMatchers.h | 9 +- 8 files changed, 258 insertions(+), 23 deletions(-) create mode 100644 libs/input/tests/InputConsumerFilteredResampling_test.cpp (limited to 'include/input') diff --git a/include/input/CoordinateFilter.h b/include/input/CoordinateFilter.h index f36472dc8c..8f2e605e85 100644 --- a/include/input/CoordinateFilter.h +++ b/include/input/CoordinateFilter.h @@ -44,7 +44,7 @@ public: * the previous call. * @param coords Coordinates to be overwritten by the corresponding filtered coordinates. */ - void filter(std::chrono::duration timestamp, PointerCoords& coords); + void filter(std::chrono::nanoseconds timestamp, PointerCoords& coords); private: OneEuroFilter mXFilter; diff --git a/include/input/OneEuroFilter.h b/include/input/OneEuroFilter.h index a0168e4f91..bdd82b2ee8 100644 --- a/include/input/OneEuroFilter.h +++ b/include/input/OneEuroFilter.h @@ -56,7 +56,7 @@ public: * provided in the previous call. * @param rawPosition Position to be filtered. */ - float filter(std::chrono::duration timestamp, float rawPosition); + float filter(std::chrono::nanoseconds timestamp, float rawPosition); private: /** @@ -67,7 +67,7 @@ private: /** * Slope of the cutoff frequency criterion. This is the term scaling the absolute value of the - * filtered signal's speed. The data member is dimensionless, that is, it does not have units. + * filtered signal's speed. Units are 1 / position. */ const float mBeta; @@ -78,9 +78,9 @@ private: const float mSpeedCutoffFreq; /** - * The timestamp from the previous call. Units are seconds. + * The timestamp from the previous call. */ - std::optional> mPrevTimestamp; + std::optional mPrevTimestamp; /** * The raw position from the previous call. @@ -88,7 +88,7 @@ private: std::optional mPrevRawPosition; /** - * The filtered velocity from the previous call. Units are position per second. + * The filtered velocity from the previous call. Units are position per nanosecond. */ std::optional mPrevFilteredVelocity; diff --git a/libs/input/CoordinateFilter.cpp b/libs/input/CoordinateFilter.cpp index d231474577..a32685bd53 100644 --- a/libs/input/CoordinateFilter.cpp +++ b/libs/input/CoordinateFilter.cpp @@ -23,7 +23,7 @@ namespace android { CoordinateFilter::CoordinateFilter(float minCutoffFreq, float beta) : mXFilter{minCutoffFreq, beta}, mYFilter{minCutoffFreq, beta} {} -void CoordinateFilter::filter(std::chrono::duration timestamp, PointerCoords& coords) { +void CoordinateFilter::filter(std::chrono::nanoseconds timestamp, PointerCoords& coords) { coords.setAxisValue(AMOTION_EVENT_AXIS_X, mXFilter.filter(timestamp, coords.getX())); coords.setAxisValue(AMOTION_EVENT_AXIS_Y, mYFilter.filter(timestamp, coords.getY())); } diff --git a/libs/input/OneEuroFilter.cpp b/libs/input/OneEuroFilter.cpp index 400d7c9ab0..7b0d104da1 100644 --- a/libs/input/OneEuroFilter.cpp +++ b/libs/input/OneEuroFilter.cpp @@ -25,16 +25,24 @@ namespace android { namespace { +using namespace std::literals::chrono_literals; + +const float kHertzPerGigahertz = 1E9f; +const float kGigahertzPerHertz = 1E-9f; + +// filteredSpeed's units are position per nanosecond. beta's units are 1 / position. inline float cutoffFreq(float minCutoffFreq, float beta, float filteredSpeed) { - return minCutoffFreq + beta * std::abs(filteredSpeed); + return kHertzPerGigahertz * + ((minCutoffFreq * kGigahertzPerHertz) + beta * std::abs(filteredSpeed)); } -inline float smoothingFactor(std::chrono::duration samplingPeriod, float cutoffFreq) { - return samplingPeriod.count() / (samplingPeriod.count() + (1.0 / (2.0 * M_PI * cutoffFreq))); +inline float smoothingFactor(std::chrono::nanoseconds samplingPeriod, float cutoffFreq) { + const float constant = 2.0f * M_PI * samplingPeriod.count() * (cutoffFreq * kGigahertzPerHertz); + return constant / (constant + 1); } -inline float lowPassFilter(float rawPosition, float prevFilteredPosition, float smoothingFactor) { - return smoothingFactor * rawPosition + (1 - smoothingFactor) * prevFilteredPosition; +inline float lowPassFilter(float rawValue, float prevFilteredValue, float smoothingFactor) { + return smoothingFactor * rawValue + (1 - smoothingFactor) * prevFilteredValue; } } // namespace @@ -42,17 +50,17 @@ inline float lowPassFilter(float rawPosition, float prevFilteredPosition, float OneEuroFilter::OneEuroFilter(float minCutoffFreq, float beta, float speedCutoffFreq) : mMinCutoffFreq{minCutoffFreq}, mBeta{beta}, mSpeedCutoffFreq{speedCutoffFreq} {} -float OneEuroFilter::filter(std::chrono::duration timestamp, float rawPosition) { - LOG_IF(FATAL, mPrevFilteredPosition.has_value() && (timestamp <= *mPrevTimestamp)) - << "Timestamp must be greater than mPrevTimestamp"; +float OneEuroFilter::filter(std::chrono::nanoseconds timestamp, float rawPosition) { + LOG_IF(FATAL, mPrevTimestamp.has_value() && (*mPrevTimestamp >= timestamp)) + << "Timestamp must be greater than mPrevTimestamp. Timestamp: " << timestamp.count() + << "ns. mPrevTimestamp: " << mPrevTimestamp->count() << "ns"; - const std::chrono::duration samplingPeriod = (mPrevTimestamp.has_value()) - ? (timestamp - *mPrevTimestamp) - : std::chrono::duration{1.0}; + const std::chrono::nanoseconds samplingPeriod = + (mPrevTimestamp.has_value()) ? (timestamp - *mPrevTimestamp) : 1s; const float rawVelocity = (mPrevFilteredPosition.has_value()) - ? ((rawPosition - *mPrevFilteredPosition) / samplingPeriod.count()) - : 0.0; + ? ((rawPosition - *mPrevFilteredPosition) / (samplingPeriod.count())) + : 0.0f; const float speedSmoothingFactor = smoothingFactor(samplingPeriod, mSpeedCutoffFreq); diff --git a/libs/input/tests/Android.bp b/libs/input/tests/Android.bp index 46e819061f..d1c564d020 100644 --- a/libs/input/tests/Android.bp +++ b/libs/input/tests/Android.bp @@ -17,6 +17,7 @@ cc_test { "IdGenerator_test.cpp", "InputChannel_test.cpp", "InputConsumer_test.cpp", + "InputConsumerFilteredResampling_test.cpp", "InputConsumerResampling_test.cpp", "InputDevice_test.cpp", "InputEvent_test.cpp", diff --git a/libs/input/tests/InputConsumerFilteredResampling_test.cpp b/libs/input/tests/InputConsumerFilteredResampling_test.cpp new file mode 100644 index 0000000000..757cd18a38 --- /dev/null +++ b/libs/input/tests/InputConsumerFilteredResampling_test.cpp @@ -0,0 +1,218 @@ +/** + * Copyright 2024 The Android Open Source Project + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +#include + +#include +#include +#include +#include + +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include + +namespace android { +namespace { + +using std::chrono::nanoseconds; + +using ::testing::AllOf; +using ::testing::Matcher; + +const int32_t ACTION_DOWN = AMOTION_EVENT_ACTION_DOWN; +const int32_t ACTION_MOVE = AMOTION_EVENT_ACTION_MOVE; + +struct Pointer { + int32_t id{0}; + ToolType toolType{ToolType::FINGER}; + float x{0.0f}; + float y{0.0f}; + bool isResampled{false}; + + PointerBuilder asPointerBuilder() const { + return PointerBuilder{id, toolType}.x(x).y(y).isResampled(isResampled); + } +}; + +} // namespace + +class InputConsumerFilteredResamplingTest : public ::testing::Test, public InputConsumerCallbacks { +protected: + InputConsumerFilteredResamplingTest() + : mClientTestChannel{std::make_shared("TestChannel")}, + mLooper{sp::make(/*allowNonCallbacks=*/false)} { + Looper::setForThread(mLooper); + mConsumer = std::make_unique< + InputConsumerNoResampling>(mClientTestChannel, mLooper, *this, []() { + return std::make_unique(/*minCutoffFreq=*/4.7, /*beta=*/0.01); + }); + } + + void invokeLooperCallback() const { + sp callback; + ASSERT_TRUE(mLooper->getFdStateDebug(mClientTestChannel->getFd(), /*ident=*/nullptr, + /*events=*/nullptr, &callback, /*data=*/nullptr)); + ASSERT_NE(callback, nullptr); + callback->handleEvent(mClientTestChannel->getFd(), ALOOPER_EVENT_INPUT, /*data=*/nullptr); + } + + void assertOnBatchedInputEventPendingWasCalled() { + ASSERT_GT(mOnBatchedInputEventPendingInvocationCount, 0UL) + << "onBatchedInputEventPending was not called"; + --mOnBatchedInputEventPendingInvocationCount; + } + + void assertReceivedMotionEvent(const Matcher& matcher) { + ASSERT_TRUE(!mMotionEvents.empty()) << "No motion events were received"; + std::unique_ptr motionEvent = std::move(mMotionEvents.front()); + mMotionEvents.pop(); + ASSERT_NE(motionEvent, nullptr) << "The consumed motion event must not be nullptr"; + EXPECT_THAT(*motionEvent, matcher); + } + + InputMessage nextPointerMessage(nanoseconds eventTime, int32_t action, const Pointer& pointer); + + std::shared_ptr mClientTestChannel; + sp mLooper; + std::unique_ptr mConsumer; + + // Batched input events + std::queue> mKeyEvents; + std::queue> mMotionEvents; + std::queue> mFocusEvents; + std::queue> mCaptureEvents; + std::queue> mDragEvents; + std::queue> mTouchModeEvents; + +private: + // InputConsumer callbacks + void onKeyEvent(std::unique_ptr event, uint32_t seq) override { + mKeyEvents.push(std::move(event)); + mConsumer->finishInputEvent(seq, /*handled=*/true); + } + + void onMotionEvent(std::unique_ptr event, uint32_t seq) override { + mMotionEvents.push(std::move(event)); + mConsumer->finishInputEvent(seq, /*handled=*/true); + } + + void onBatchedInputEventPending(int32_t pendingBatchSource) override { + if (!mConsumer->probablyHasInput()) { + ADD_FAILURE() << "Should deterministically have input because there is a batch"; + } + ++mOnBatchedInputEventPendingInvocationCount; + } + + void onFocusEvent(std::unique_ptr event, uint32_t seq) override { + mFocusEvents.push(std::move(event)); + mConsumer->finishInputEvent(seq, /*handled=*/true); + } + + void onCaptureEvent(std::unique_ptr event, uint32_t seq) override { + mCaptureEvents.push(std::move(event)); + mConsumer->finishInputEvent(seq, /*handled=*/true); + } + + void onDragEvent(std::unique_ptr event, uint32_t seq) override { + mDragEvents.push(std::move(event)); + mConsumer->finishInputEvent(seq, /*handled=*/true); + } + + void onTouchModeEvent(std::unique_ptr event, uint32_t seq) override { + mTouchModeEvents.push(std::move(event)); + mConsumer->finishInputEvent(seq, /*handled=*/true); + } + + uint32_t mLastSeq{0}; + size_t mOnBatchedInputEventPendingInvocationCount{0}; +}; + +InputMessage InputConsumerFilteredResamplingTest::nextPointerMessage(nanoseconds eventTime, + int32_t action, + const Pointer& pointer) { + ++mLastSeq; + return InputMessageBuilder{InputMessage::Type::MOTION, mLastSeq} + .eventTime(eventTime.count()) + .source(AINPUT_SOURCE_TOUCHSCREEN) + .action(action) + .pointer(pointer.asPointerBuilder()) + .build(); +} + +TEST_F(InputConsumerFilteredResamplingTest, NeighboringTimestampsDoNotResultInZeroDivision) { + mClientTestChannel->enqueueMessage( + nextPointerMessage(0ms, ACTION_DOWN, Pointer{.x = 0.0f, .y = 0.0f})); + + invokeLooperCallback(); + + assertReceivedMotionEvent(AllOf(WithMotionAction(ACTION_DOWN), WithSampleCount(1))); + + const std::chrono::nanoseconds initialTime{56'821'700'000'000}; + + mClientTestChannel->enqueueMessage(nextPointerMessage(initialTime + 4'929'000ns, ACTION_MOVE, + Pointer{.x = 1.0f, .y = 1.0f})); + mClientTestChannel->enqueueMessage(nextPointerMessage(initialTime + 9'352'000ns, ACTION_MOVE, + Pointer{.x = 2.0f, .y = 2.0f})); + mClientTestChannel->enqueueMessage(nextPointerMessage(initialTime + 14'531'000ns, ACTION_MOVE, + Pointer{.x = 3.0f, .y = 3.0f})); + + invokeLooperCallback(); + mConsumer->consumeBatchedInputEvents(initialTime.count() + 18'849'395 /*ns*/); + + assertOnBatchedInputEventPendingWasCalled(); + // Three samples are expected. The first two of the batch, and the resampled one. The + // coordinates of the resampled sample are hardcoded because the matcher requires them. However, + // the primary intention here is to check that the last sample is resampled. + assertReceivedMotionEvent(AllOf(WithMotionAction(ACTION_MOVE), WithSampleCount(3), + WithSample(/*sampleIndex=*/2, + Sample{initialTime + 13'849'395ns, + {PointerArgs{.x = 1.3286f, + .y = 1.3286f, + .isResampled = true}}}))); + + mClientTestChannel->enqueueMessage(nextPointerMessage(initialTime + 20'363'000ns, ACTION_MOVE, + Pointer{.x = 4.0f, .y = 4.0f})); + mClientTestChannel->enqueueMessage(nextPointerMessage(initialTime + 25'745'000ns, ACTION_MOVE, + Pointer{.x = 5.0f, .y = 5.0f})); + // This sample is part of the stream of messages, but should not be consumed because its + // timestamp is greater than the ajusted frame time. + mClientTestChannel->enqueueMessage(nextPointerMessage(initialTime + 31'337'000ns, ACTION_MOVE, + Pointer{.x = 6.0f, .y = 6.0f})); + + invokeLooperCallback(); + mConsumer->consumeBatchedInputEvents(initialTime.count() + 35'516'062 /*ns*/); + + assertOnBatchedInputEventPendingWasCalled(); + // Four samples are expected because the last sample of the previous batch was not consumed. + assertReceivedMotionEvent(AllOf(WithMotionAction(ACTION_MOVE), WithSampleCount(4))); + + mClientTestChannel->assertFinishMessage(/*seq=*/1, /*handled=*/true); + mClientTestChannel->assertFinishMessage(/*seq=*/2, /*handled=*/true); + mClientTestChannel->assertFinishMessage(/*seq=*/3, /*handled=*/true); + mClientTestChannel->assertFinishMessage(/*seq=*/4, /*handled=*/true); + mClientTestChannel->assertFinishMessage(/*seq=*/5, /*handled=*/true); + mClientTestChannel->assertFinishMessage(/*seq=*/6, /*handled=*/true); +} + +} // namespace android diff --git a/libs/input/tests/OneEuroFilter_test.cpp b/libs/input/tests/OneEuroFilter_test.cpp index 270e789c84..8645508ea7 100644 --- a/libs/input/tests/OneEuroFilter_test.cpp +++ b/libs/input/tests/OneEuroFilter_test.cpp @@ -98,7 +98,10 @@ protected: std::vector filteredSignal; for (const Sample& sample : signal) { filteredSignal.push_back( - Sample{sample.timestamp, mFilter.filter(sample.timestamp, sample.value)}); + Sample{sample.timestamp, + mFilter.filter(std::chrono::duration_cast( + sample.timestamp), + sample.value)}); } return filteredSignal; } diff --git a/libs/input/tests/TestEventMatchers.h b/libs/input/tests/TestEventMatchers.h index 3589de599f..de96600f66 100644 --- a/libs/input/tests/TestEventMatchers.h +++ b/libs/input/tests/TestEventMatchers.h @@ -17,6 +17,7 @@ #pragma once #include +#include #include #include @@ -150,14 +151,18 @@ public: ++pointerIndex) { const PointerCoords& pointerCoords = *(motionEvent.getHistoricalRawPointerCoords(pointerIndex, mSampleIndex)); - if ((pointerCoords.getX() != mSample.pointers[pointerIndex].x) || - (pointerCoords.getY() != mSample.pointers[pointerIndex].y)) { + + if ((std::abs(pointerCoords.getX() - mSample.pointers[pointerIndex].x) > + MotionEvent::ROUNDING_PRECISION) || + (std::abs(pointerCoords.getY() - mSample.pointers[pointerIndex].y) > + MotionEvent::ROUNDING_PRECISION)) { *os << "sample coordinates mismatch at pointer index " << pointerIndex << ". sample: (" << pointerCoords.getX() << ", " << pointerCoords.getY() << ") expected: (" << mSample.pointers[pointerIndex].x << ", " << mSample.pointers[pointerIndex].y << ")"; return false; } + if (motionEvent.isResampled(pointerIndex, mSampleIndex) != mSample.pointers[pointerIndex].isResampled) { *os << "resampling flag mismatch. sample: " -- cgit v1.2.3-59-g8ed1b From ff63fe16c5302683d12a1e1f0939233a19875c76 Mon Sep 17 00:00:00 2001 From: Harry Cutts Date: Fri, 22 Nov 2024 17:45:25 +0000 Subject: input: Remove prefix from InputDeviceSensorAccuracy enum This is an enum class, so the value identifiers aren't in the global namespace and therefore don't need to be Smurf-named. Bug: 245989146 Change-Id: I7ec70dad5651eddcb55f61c3ebaeffc2e31f251d Test: m checkinput Flag: EXEMPT refactor --- include/input/InputDevice.h | 10 +++++----- services/inputflinger/reader/mapper/SensorInputMapper.cpp | 2 +- services/inputflinger/reader/mapper/SensorInputMapper.h | 2 +- services/inputflinger/tests/SensorInputMapper_test.cpp | 4 ++-- 4 files changed, 9 insertions(+), 9 deletions(-) (limited to 'include/input') diff --git a/include/input/InputDevice.h b/include/input/InputDevice.h index 6b45dd39dc..ea1e4aee01 100644 --- a/include/input/InputDevice.h +++ b/include/input/InputDevice.h @@ -111,12 +111,12 @@ enum class InputDeviceSensorType : int32_t { }; enum class InputDeviceSensorAccuracy : int32_t { - ACCURACY_NONE = 0, - ACCURACY_LOW = 1, - ACCURACY_MEDIUM = 2, - ACCURACY_HIGH = 3, + NONE = 0, + LOW = 1, + MEDIUM = 2, + HIGH = 3, - ftl_last = ACCURACY_HIGH, + ftl_last = HIGH, }; enum class InputDeviceSensorReportingMode : int32_t { diff --git a/services/inputflinger/reader/mapper/SensorInputMapper.cpp b/services/inputflinger/reader/mapper/SensorInputMapper.cpp index 4233f789d6..1f6600d3f6 100644 --- a/services/inputflinger/reader/mapper/SensorInputMapper.cpp +++ b/services/inputflinger/reader/mapper/SensorInputMapper.cpp @@ -212,7 +212,7 @@ SensorInputMapper::Sensor SensorInputMapper::createSensor(InputDeviceSensorType // One input device can only have 1 sensor for each sensor Type. InputDeviceSensorInfo sensorInfo(identifier.name, std::to_string(identifier.vendor), identifier.version, sensorType, - InputDeviceSensorAccuracy::ACCURACY_HIGH, + InputDeviceSensorAccuracy::HIGH, /*maxRange=*/axis.max, /*resolution=*/axis.scale, /*power=*/config.getFloat(prefix + ".power").value_or(0.0f), /*minDelay=*/config.getInt(prefix + ".minDelay").value_or(0), diff --git a/services/inputflinger/reader/mapper/SensorInputMapper.h b/services/inputflinger/reader/mapper/SensorInputMapper.h index 63bc151ac1..7974efe172 100644 --- a/services/inputflinger/reader/mapper/SensorInputMapper.h +++ b/services/inputflinger/reader/mapper/SensorInputMapper.h @@ -101,7 +101,7 @@ private: std::array dataVec; void resetValue() { this->enabled = false; - this->accuracy = InputDeviceSensorAccuracy::ACCURACY_NONE; + this->accuracy = InputDeviceSensorAccuracy::NONE; this->samplingPeriod = std::chrono::nanoseconds(0); this->maxBatchReportLatency = std::chrono::nanoseconds(0); this->lastSampleTimeNs = std::nullopt; diff --git a/services/inputflinger/tests/SensorInputMapper_test.cpp b/services/inputflinger/tests/SensorInputMapper_test.cpp index 2c1265378a..ac2e99e7b5 100644 --- a/services/inputflinger/tests/SensorInputMapper_test.cpp +++ b/services/inputflinger/tests/SensorInputMapper_test.cpp @@ -125,7 +125,7 @@ TEST_F(SensorInputMapperTest, ProcessAccelerometerSensor) { ASSERT_EQ(arg.source, AINPUT_SOURCE_SENSOR); ASSERT_EQ(arg.deviceId, DEVICE_ID); ASSERT_EQ(arg.sensorType, InputDeviceSensorType::ACCELEROMETER); - ASSERT_EQ(arg.accuracy, InputDeviceSensorAccuracy::ACCURACY_HIGH); + ASSERT_EQ(arg.accuracy, InputDeviceSensorAccuracy::HIGH); ASSERT_EQ(arg.hwTimestamp, ARBITRARY_TIME); ASSERT_EQ(arg.values, values); mMapper->flushSensor(InputDeviceSensorType::ACCELEROMETER); @@ -170,7 +170,7 @@ TEST_F(SensorInputMapperTest, ProcessGyroscopeSensor) { ASSERT_EQ(arg.source, AINPUT_SOURCE_SENSOR); ASSERT_EQ(arg.deviceId, DEVICE_ID); ASSERT_EQ(arg.sensorType, InputDeviceSensorType::GYROSCOPE); - ASSERT_EQ(arg.accuracy, InputDeviceSensorAccuracy::ACCURACY_HIGH); + ASSERT_EQ(arg.accuracy, InputDeviceSensorAccuracy::HIGH); ASSERT_EQ(arg.hwTimestamp, ARBITRARY_TIME); ASSERT_EQ(arg.values, values); mMapper->flushSensor(InputDeviceSensorType::GYROSCOPE); -- cgit v1.2.3-59-g8ed1b From 9a14c1202f79c26b4bb67807dd617a19d0c380c3 Mon Sep 17 00:00:00 2001 From: Oleg Blinnikov Date: Wed, 23 Oct 2024 17:30:03 +0000 Subject: DisplayTopology propagation to IM Change-Id: Ic0cf7f798175c7005d1bd0a57d116ef9542cd7a6 Bug: 364907904 Test: builds Flag: com.android.server.display.feature.flags.display_topology --- include/input/DisplayTopologyGraph.h | 57 ++++++++++++++++++++++++++++++++++++ 1 file changed, 57 insertions(+) create mode 100644 include/input/DisplayTopologyGraph.h (limited to 'include/input') diff --git a/include/input/DisplayTopologyGraph.h b/include/input/DisplayTopologyGraph.h new file mode 100644 index 0000000000..90427bd76d --- /dev/null +++ b/include/input/DisplayTopologyGraph.h @@ -0,0 +1,57 @@ +/* + * Copyright 2024 The Android Open Source Project + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +#pragma once + +#include +#include + +#include +#include +#include + +namespace android { + +/** + * The edge of the current display, where adjacent display is attached to. + */ +enum class DisplayTopologyPosition : int32_t { + LEFT = 0, + TOP = 1, + RIGHT = 2, + BOTTOM = 3, + + ftl_last = BOTTOM +}; + +/** + * Directed edge in the graph of adjacent displays. + */ +struct DisplayTopologyAdjacentDisplay { + ui::LogicalDisplayId displayId = ui::LogicalDisplayId::INVALID; + DisplayTopologyPosition position; + float offsetPx; +}; + +/** + * Directed Graph representation of Display Topology. + */ +struct DisplayTopologyGraph { + ui::LogicalDisplayId primaryDisplayId = ui::LogicalDisplayId::INVALID; + std::unordered_map> graph; +}; + +} // namespace android -- cgit v1.2.3-59-g8ed1b