From 6573583a3b304dce8ff334df8d48d01b58ab2c84 Mon Sep 17 00:00:00 2001 From: Siarhei Vishniakou Date: Tue, 9 Aug 2022 19:18:37 +0000 Subject: Allow stylus events in PalmRejector After some recent changes, the touchscreen input device has source that is a combination of SOURCE_TOUCHSCREEN and SOURCE_STYLUS. Before this CL, this source is rejected, and therefore palm rejection feature is not enabled. With this CL, any source that has SOURCE_TOUCHSCREEN is allowed. That also means that we potentially would invoke the model for stylus events, especially if simultaneous touch and stylus are enabled. The model, however, was never trained on stylus, and is not designed to work with it. In preparation to upcoming simultaneous touch+stylus feature, in this CL we also remove the stylus pointers before sending the data to the model. The only case where we need to be careful is pointer-down and pointer-up events with stylus. In this CL, we drop these events, which should be a no-op from the palm rejection model's perpective. Bug: 241935838 Test: atest libpalmrejection_test inputflinger_tests Change-Id: I760207a82807e03802e72a318fca8b97a4fd7a24 --- .../inputflinger/UnwantedInteractionBlocker.cpp | 50 +++++++----- services/inputflinger/UnwantedInteractionBlocker.h | 19 +++++ .../tests/UnwantedInteractionBlocker_test.cpp | 88 ++++++++++++++++++++++ 3 files changed, 137 insertions(+), 20 deletions(-) diff --git a/services/inputflinger/UnwantedInteractionBlocker.cpp b/services/inputflinger/UnwantedInteractionBlocker.cpp index fc8dfe678b..ec41025e9d 100644 --- a/services/inputflinger/UnwantedInteractionBlocker.cpp +++ b/services/inputflinger/UnwantedInteractionBlocker.cpp @@ -77,8 +77,7 @@ static std::string toLower(std::string s) { } static bool isFromTouchscreen(int32_t source) { - return isFromSource(source, AINPUT_SOURCE_TOUCHSCREEN) && - !isFromSource(source, AINPUT_SOURCE_STYLUS); + return isFromSource(source, AINPUT_SOURCE_TOUCHSCREEN); } static ::base::TimeTicks toChromeTimestamp(nsecs_t eventTime) { @@ -142,23 +141,6 @@ static int32_t resolveActionForPointer(uint8_t pointerIndex, int32_t action) { return AMOTION_EVENT_ACTION_MOVE; } -/** - * Remove the data for the provided pointers from the args. The pointers are identified by their - * pointerId, not by the index inside the array. - * Return the new NotifyMotionArgs struct that has the remaining pointers. - * The only fields that may be different in the returned args from the provided args are: - * - action - * - pointerCount - * - pointerProperties - * - pointerCoords - * Action might change because it contains a pointer index. If another pointer is removed, the - * active pointer index would be shifted. - * Do not call this function for events with POINTER_UP or POINTER_DOWN events when removed pointer - * id is the acting pointer id. - * - * @param args the args from which the pointers should be removed - * @param pointerIds the pointer ids of the pointers that should be removed - */ NotifyMotionArgs removePointerIds(const NotifyMotionArgs& args, const std::set& pointerIds) { const uint8_t actionIndex = MotionEvent::getActionIndex(args.action); @@ -204,6 +186,26 @@ NotifyMotionArgs removePointerIds(const NotifyMotionArgs& args, return newArgs; } +/** + * Remove stylus pointers from the provided NotifyMotionArgs. + * + * Return NotifyMotionArgs where the stylus pointers have been removed. + * If this results in removal of the active pointer, then return nullopt. + */ +static std::optional removeStylusPointerIds(const NotifyMotionArgs& args) { + std::set stylusPointerIds; + for (uint32_t i = 0; i < args.pointerCount; i++) { + if (args.pointerProperties[i].toolType == AMOTION_EVENT_TOOL_TYPE_STYLUS) { + stylusPointerIds.insert(args.pointerProperties[i].id); + } + } + NotifyMotionArgs withoutStylusPointers = removePointerIds(args, stylusPointerIds); + if (withoutStylusPointers.pointerCount == 0 || withoutStylusPointers.action == ACTION_UNKNOWN) { + return std::nullopt; + } + return withoutStylusPointers; +} + std::optional createPalmFilterDeviceInfo( const InputDeviceInfo& deviceInfo) { if (!isFromTouchscreen(deviceInfo.getSources())) { @@ -678,7 +680,15 @@ std::vector PalmRejector::processMotion(const NotifyMotionArgs std::set oldSuppressedIds; std::swap(oldSuppressedIds, mSuppressedPointerIds); - mSuppressedPointerIds = detectPalmPointers(args); + + std::optional touchOnlyArgs = removeStylusPointerIds(args); + if (touchOnlyArgs) { + mSuppressedPointerIds = detectPalmPointers(*touchOnlyArgs); + } else { + // This is a stylus-only event. + // We can skip this event and just keep the suppressed pointer ids the same as before. + mSuppressedPointerIds = oldSuppressedIds; + } std::vector argsWithoutUnwantedPointers = cancelSuppressedPointers(args, oldSuppressedIds, mSuppressedPointerIds); diff --git a/services/inputflinger/UnwantedInteractionBlocker.h b/services/inputflinger/UnwantedInteractionBlocker.h index a176a985c2..5d0dde8e64 100644 --- a/services/inputflinger/UnwantedInteractionBlocker.h +++ b/services/inputflinger/UnwantedInteractionBlocker.h @@ -43,6 +43,25 @@ std::optional createPalmFilterDeviceInfo( static constexpr int32_t ACTION_UNKNOWN = -1; +/** + * Remove the data for the provided pointers from the args. The pointers are identified by their + * pointerId, not by the index inside the array. + * Return the new NotifyMotionArgs struct that has the remaining pointers. + * The only fields that may be different in the returned args from the provided args are: + * - action + * - pointerCount + * - pointerProperties + * - pointerCoords + * Action might change because it contains a pointer index. If another pointer is removed, the + * active pointer index would be shifted. + * + * If the active pointer id is removed (for example, for events like + * POINTER_UP or POINTER_DOWN), then the action is set to ACTION_UNKNOWN. It is up to the caller + * to set the action appropriately after the call. + * + * @param args the args from which the pointers should be removed + * @param pointerIds the pointer ids of the pointers that should be removed + */ NotifyMotionArgs removePointerIds(const NotifyMotionArgs& args, const std::set& pointerIds); diff --git a/services/inputflinger/tests/UnwantedInteractionBlocker_test.cpp b/services/inputflinger/tests/UnwantedInteractionBlocker_test.cpp index 9313a4500d..29fa001185 100644 --- a/services/inputflinger/tests/UnwantedInteractionBlocker_test.cpp +++ b/services/inputflinger/tests/UnwantedInteractionBlocker_test.cpp @@ -66,6 +66,10 @@ MATCHER_P(WithAction, action, "MotionEvent with specified action") { return result; } +MATCHER_P(WithFlags, flags, "MotionEvent with specified flags") { + return arg.flags == flags; +} + static nsecs_t toNs(std::chrono::nanoseconds duration) { return duration.count(); } @@ -616,6 +620,90 @@ TEST_F(UnwantedInteractionBlockerTest, HeuristicFilterWorks) { mTestListener.assertNotifyMotionWasCalled(WithAction(CANCEL)); } +/** + * Send a stylus event that would have triggered the heuristic palm detector if it were a touch + * event. However, since it's a stylus event, it should propagate without being canceled through + * the blocker. + * This is similar to `HeuristicFilterWorks` test, but for stylus tool. + */ +TEST_F(UnwantedInteractionBlockerTest, StylusIsNotBlocked) { + InputDeviceInfo info = generateTestDeviceInfo(); + info.addSource(AINPUT_SOURCE_STYLUS); + mBlocker->notifyInputDevicesChanged({info}); + NotifyMotionArgs args1 = generateMotionArgs(0 /*downTime*/, 0 /*eventTime*/, DOWN, {{1, 2, 3}}); + args1.pointerProperties[0].toolType = AMOTION_EVENT_TOOL_TYPE_STYLUS; + mBlocker->notifyMotion(&args1); + mTestListener.assertNotifyMotionWasCalled(WithAction(DOWN)); + + // Move the stylus, setting large TOUCH_MAJOR/TOUCH_MINOR dimensions + NotifyMotionArgs args2 = + generateMotionArgs(0 /*downTime*/, RESAMPLE_PERIOD, MOVE, {{4, 5, 200}}); + args2.pointerProperties[0].toolType = AMOTION_EVENT_TOOL_TYPE_STYLUS; + mBlocker->notifyMotion(&args2); + mTestListener.assertNotifyMotionWasCalled(WithAction(MOVE)); + + // Lift up the stylus. If it were a touch event, this would force the model to decide on whether + // it's a palm. + NotifyMotionArgs args3 = + generateMotionArgs(0 /*downTime*/, 2 * RESAMPLE_PERIOD, UP, {{4, 5, 200}}); + args3.pointerProperties[0].toolType = AMOTION_EVENT_TOOL_TYPE_STYLUS; + mBlocker->notifyMotion(&args3); + mTestListener.assertNotifyMotionWasCalled(WithAction(UP)); +} + +/** + * Send a mixed touch and stylus event. + * The touch event goes first, and is a palm. The stylus event goes down after. + * Stylus event should continue to work even after touch is detected as a palm. + */ +TEST_F(UnwantedInteractionBlockerTest, TouchIsBlockedWhenMixedWithStylus) { + InputDeviceInfo info = generateTestDeviceInfo(); + info.addSource(AINPUT_SOURCE_STYLUS); + mBlocker->notifyInputDevicesChanged({info}); + + // Touch down + NotifyMotionArgs args1 = generateMotionArgs(0 /*downTime*/, 0 /*eventTime*/, DOWN, {{1, 2, 3}}); + mBlocker->notifyMotion(&args1); + mTestListener.assertNotifyMotionWasCalled(WithAction(DOWN)); + + // Stylus pointer down + NotifyMotionArgs args2 = generateMotionArgs(0 /*downTime*/, RESAMPLE_PERIOD, POINTER_1_DOWN, + {{1, 2, 3}, {10, 20, 30}}); + args2.pointerProperties[1].toolType = AMOTION_EVENT_TOOL_TYPE_STYLUS; + mBlocker->notifyMotion(&args2); + mTestListener.assertNotifyMotionWasCalled(WithAction(POINTER_1_DOWN)); + + // Large touch oval on the next finger move + NotifyMotionArgs args3 = generateMotionArgs(0 /*downTime*/, 2 * RESAMPLE_PERIOD, MOVE, + {{1, 2, 300}, {11, 21, 30}}); + args3.pointerProperties[1].toolType = AMOTION_EVENT_TOOL_TYPE_STYLUS; + mBlocker->notifyMotion(&args3); + mTestListener.assertNotifyMotionWasCalled(WithAction(MOVE)); + + // Lift up the finger pointer. It should be canceled due to the heuristic filter. + NotifyMotionArgs args4 = generateMotionArgs(0 /*downTime*/, 3 * RESAMPLE_PERIOD, POINTER_0_UP, + {{1, 2, 300}, {11, 21, 30}}); + args4.pointerProperties[1].toolType = AMOTION_EVENT_TOOL_TYPE_STYLUS; + mBlocker->notifyMotion(&args4); + mTestListener.assertNotifyMotionWasCalled( + AllOf(WithAction(POINTER_0_UP), WithFlags(FLAG_CANCELED))); + + NotifyMotionArgs args5 = + generateMotionArgs(0 /*downTime*/, 4 * RESAMPLE_PERIOD, MOVE, {{12, 22, 30}}); + args5.pointerProperties[0].id = args4.pointerProperties[1].id; + args5.pointerProperties[0].toolType = AMOTION_EVENT_TOOL_TYPE_STYLUS; + mBlocker->notifyMotion(&args5); + mTestListener.assertNotifyMotionWasCalled(WithAction(MOVE)); + + // Lift up the stylus pointer + NotifyMotionArgs args6 = + generateMotionArgs(0 /*downTime*/, 5 * RESAMPLE_PERIOD, UP, {{4, 5, 200}}); + args6.pointerProperties[0].id = args4.pointerProperties[1].id; + args6.pointerProperties[0].toolType = AMOTION_EVENT_TOOL_TYPE_STYLUS; + mBlocker->notifyMotion(&args6); + mTestListener.assertNotifyMotionWasCalled(WithAction(UP)); +} + using UnwantedInteractionBlockerTestDeathTest = UnwantedInteractionBlockerTest; /** -- cgit v1.2.3-59-g8ed1b