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
diff --git a/services/inputflinger/UnwantedInteractionBlocker.cpp b/services/inputflinger/UnwantedInteractionBlocker.cpp
index fc8dfe6..ec41025 100644
--- a/services/inputflinger/UnwantedInteractionBlocker.cpp
+++ b/services/inputflinger/UnwantedInteractionBlocker.cpp
@@ -77,8 +77,7 @@
}
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 @@
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<int32_t>& pointerIds) {
const uint8_t actionIndex = MotionEvent::getActionIndex(args.action);
@@ -204,6 +186,26 @@
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<NotifyMotionArgs> removeStylusPointerIds(const NotifyMotionArgs& args) {
+ std::set<int32_t> 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<AndroidPalmFilterDeviceInfo> createPalmFilterDeviceInfo(
const InputDeviceInfo& deviceInfo) {
if (!isFromTouchscreen(deviceInfo.getSources())) {
@@ -678,7 +680,15 @@
std::set<int32_t> oldSuppressedIds;
std::swap(oldSuppressedIds, mSuppressedPointerIds);
- mSuppressedPointerIds = detectPalmPointers(args);
+
+ std::optional<NotifyMotionArgs> 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<NotifyMotionArgs> argsWithoutUnwantedPointers =
cancelSuppressedPointers(args, oldSuppressedIds, mSuppressedPointerIds);
diff --git a/services/inputflinger/UnwantedInteractionBlocker.h b/services/inputflinger/UnwantedInteractionBlocker.h
index a176a98..5d0dde8 100644
--- a/services/inputflinger/UnwantedInteractionBlocker.h
+++ b/services/inputflinger/UnwantedInteractionBlocker.h
@@ -43,6 +43,25 @@
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<int32_t>& pointerIds);
diff --git a/services/inputflinger/tests/UnwantedInteractionBlocker_test.cpp b/services/inputflinger/tests/UnwantedInteractionBlocker_test.cpp
index 9313a45..29fa001 100644
--- a/services/inputflinger/tests/UnwantedInteractionBlocker_test.cpp
+++ b/services/inputflinger/tests/UnwantedInteractionBlocker_test.cpp
@@ -66,6 +66,10 @@
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 @@
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;
/**