Add delay before enabling tap while typing on physical keyboard
To improve touchpad palm rejection while typing, this change will add a
delay before tap to click is enabled.
This change add a delay to 400ms, this is based on internal testing
within team, and may be updated after a cafe study with large audiance.
Bug: 301055381
Test: atest inputflinger_tests
Change-Id: Ie582b5932a8bbcdbf479d375a25e16a1c8318253
diff --git a/services/inputflinger/reader/mapper/gestures/GestureConverter.cpp b/services/inputflinger/reader/mapper/gestures/GestureConverter.cpp
index 71cb5a7..b36a94d 100644
--- a/services/inputflinger/reader/mapper/gestures/GestureConverter.cpp
+++ b/services/inputflinger/reader/mapper/gestures/GestureConverter.cpp
@@ -81,6 +81,7 @@
out << StringPrintf("Button state: 0x%08x\n", mButtonState);
out << "Down time: " << mDownTime << "\n";
out << "Current classification: " << ftl::enum_string(mCurrentClassification) << "\n";
+ out << "Enable Tap Timestamp: " << mWhenToEnableTapToClick << "\n";
return out.str();
}
@@ -196,8 +197,9 @@
if (!mIsHoverCancelled) {
// handleFling calls hoverMove with zero delta on FLING_TAP_DOWN. Don't enable tap to click
// for this case as subsequent handleButtonsChange may choose to ignore this tap.
- if (ENABLE_TOUCHPAD_PALM_REJECTION && (std::abs(deltaX) > 0 || std::abs(deltaY) > 0)) {
- enableTapToClick();
+ if ((ENABLE_TOUCHPAD_PALM_REJECTION || ENABLE_TOUCHPAD_PALM_REJECTION_V2) &&
+ (std::abs(deltaX) > 0 || std::abs(deltaY) > 0)) {
+ enableTapToClick(when);
}
mPointerController->setPresentation(PointerControllerInterface::Presentation::POINTER);
mPointerController->move(deltaX, deltaY);
@@ -241,8 +243,15 @@
coords.setAxisValue(AMOTION_EVENT_AXIS_RELATIVE_X, 0);
coords.setAxisValue(AMOTION_EVENT_AXIS_RELATIVE_Y, 0);
- if (ENABLE_TOUCHPAD_PALM_REJECTION && mReaderContext.isPreventingTouchpadTaps()) {
- enableTapToClick();
+ // V2 palm rejection should override V1
+ if (ENABLE_TOUCHPAD_PALM_REJECTION_V2) {
+ enableTapToClick(when);
+ if (gesture.details.buttons.is_tap && when <= mWhenToEnableTapToClick) {
+ // return early to prevent this tap
+ return out;
+ }
+ } else if (ENABLE_TOUCHPAD_PALM_REJECTION && mReaderContext.isPreventingTouchpadTaps()) {
+ enableTapToClick(when);
if (gesture.details.buttons.is_tap) {
// return early to prevent this tap
return out;
@@ -399,7 +408,7 @@
// TODO(b/282023644): Add a signal in libgestures for when a stable contact has been
// initiated with a touchpad.
if (!mReaderContext.isPreventingTouchpadTaps()) {
- enableTapToClick();
+ enableTapToClick(when);
}
return handleMove(when, readTime, gestureStartTime,
Gesture(kGestureMove, gesture.start_time, gesture.end_time,
@@ -630,8 +639,11 @@
/* videoFrames= */ {}};
}
-void GestureConverter::enableTapToClick() {
- mReaderContext.setPreventingTouchpadTaps(false);
+void GestureConverter::enableTapToClick(nsecs_t when) {
+ if (mReaderContext.isPreventingTouchpadTaps()) {
+ mWhenToEnableTapToClick = when + TAP_ENABLE_DELAY_NANOS.count();
+ mReaderContext.setPreventingTouchpadTaps(false);
+ }
}
} // namespace android
diff --git a/services/inputflinger/reader/mapper/gestures/GestureConverter.h b/services/inputflinger/reader/mapper/gestures/GestureConverter.h
index c51a586..88e7b99 100644
--- a/services/inputflinger/reader/mapper/gestures/GestureConverter.h
+++ b/services/inputflinger/reader/mapper/gestures/GestureConverter.h
@@ -34,6 +34,13 @@
namespace android {
+using std::chrono_literals::operator""ms;
+/**
+ * This duration is decided based on internal team testing, it may be updated after testing with
+ * larger groups
+ */
+constexpr std::chrono::nanoseconds TAP_ENABLE_DELAY_NANOS = 400ms;
+
// Converts Gesture structs from the gestures library into NotifyArgs and the appropriate
// PointerController calls.
class GestureConverter {
@@ -85,8 +92,9 @@
const PointerCoords* pointerCoords, float xCursorPosition,
float yCursorPosition);
- void enableTapToClick();
+ void enableTapToClick(nsecs_t when);
bool mIsHoverCancelled{false};
+ nsecs_t mWhenToEnableTapToClick{0};
const int32_t mDeviceId;
InputReaderContext& mReaderContext;
diff --git a/services/inputflinger/tests/GestureConverter_test.cpp b/services/inputflinger/tests/GestureConverter_test.cpp
index a80bbfd..7be6d46 100644
--- a/services/inputflinger/tests/GestureConverter_test.cpp
+++ b/services/inputflinger/tests/GestureConverter_test.cpp
@@ -1202,7 +1202,10 @@
}
TEST_F_WITH_FLAGS(GestureConverterTest, TapWithTapToClickDisabled,
- REQUIRES_FLAGS_ENABLED(TOUCHPAD_PALM_REJECTION)) {
+ REQUIRES_FLAGS_ENABLED(TOUCHPAD_PALM_REJECTION),
+ REQUIRES_FLAGS_DISABLED(TOUCHPAD_PALM_REJECTION_V2)) {
+ nsecs_t currentTime = ARBITRARY_GESTURE_TIME;
+
// Tap should be ignored when disabled
mReader->getContext()->setPreventingTouchpadTaps(true);
@@ -1210,10 +1213,10 @@
GestureConverter converter(*mReader->getContext(), deviceContext, DEVICE_ID);
converter.setDisplayId(ADISPLAY_ID_DEFAULT);
- Gesture flingGesture(kGestureFling, ARBITRARY_GESTURE_TIME, ARBITRARY_GESTURE_TIME, /* vx= */ 0,
+ Gesture flingGesture(kGestureFling, currentTime, currentTime, /* vx= */ 0,
/* vy= */ 0, GESTURES_FLING_TAP_DOWN);
std::list<NotifyArgs> args =
- converter.handleGesture(ARBITRARY_TIME, READ_TIME, ARBITRARY_TIME, flingGesture);
+ converter.handleGesture(currentTime, currentTime, currentTime, flingGesture);
ASSERT_EQ(1u, args.size());
ASSERT_THAT(std::get<NotifyMotionArgs>(args.front()),
@@ -1221,12 +1224,11 @@
WithCoords(POINTER_X, POINTER_Y), WithRelativeMotion(0, 0),
WithToolType(ToolType::FINGER), WithButtonState(0), WithPressure(0.0f),
WithDisplayId(ADISPLAY_ID_DEFAULT)));
- args.pop_front();
- Gesture tapGesture(kGestureButtonsChange, ARBITRARY_GESTURE_TIME, ARBITRARY_GESTURE_TIME,
+ Gesture tapGesture(kGestureButtonsChange, currentTime, currentTime,
/* down= */ GESTURES_BUTTON_LEFT,
/* up= */ GESTURES_BUTTON_LEFT, /* is_tap= */ true);
- args = converter.handleGesture(ARBITRARY_TIME, READ_TIME, ARBITRARY_TIME, tapGesture);
+ args = converter.handleGesture(currentTime, currentTime, currentTime, tapGesture);
// no events should be generated
ASSERT_EQ(0u, args.size());
@@ -1235,6 +1237,97 @@
ASSERT_FALSE(mReader->getContext()->isPreventingTouchpadTaps());
}
+TEST_F_WITH_FLAGS(GestureConverterTest, TapWithTapToClickDisabledWithDelay,
+ REQUIRES_FLAGS_ENABLED(TOUCHPAD_PALM_REJECTION_V2)) {
+ nsecs_t currentTime = ARBITRARY_GESTURE_TIME;
+
+ // Tap should be ignored when disabled
+ mReader->getContext()->setPreventingTouchpadTaps(true);
+
+ InputDeviceContext deviceContext(*mDevice, EVENTHUB_ID);
+ GestureConverter converter(*mReader->getContext(), deviceContext, DEVICE_ID);
+ converter.setDisplayId(ADISPLAY_ID_DEFAULT);
+
+ Gesture flingGesture(kGestureFling, currentTime, currentTime, /* vx= */ 0,
+ /* vy= */ 0, GESTURES_FLING_TAP_DOWN);
+ std::list<NotifyArgs> args =
+ converter.handleGesture(currentTime, currentTime, currentTime, flingGesture);
+
+ ASSERT_EQ(1u, args.size());
+ ASSERT_THAT(std::get<NotifyMotionArgs>(args.front()),
+ AllOf(WithMotionAction(AMOTION_EVENT_ACTION_HOVER_MOVE),
+ WithCoords(POINTER_X, POINTER_Y), WithRelativeMotion(0, 0),
+ WithToolType(ToolType::FINGER), WithButtonState(0), WithPressure(0.0f),
+ WithDisplayId(ADISPLAY_ID_DEFAULT)));
+
+ Gesture tapGesture(kGestureButtonsChange, currentTime, currentTime,
+ /* down= */ GESTURES_BUTTON_LEFT,
+ /* up= */ GESTURES_BUTTON_LEFT, /* is_tap= */ true);
+ args = converter.handleGesture(currentTime, currentTime, currentTime, tapGesture);
+
+ // no events should be generated
+ ASSERT_EQ(0u, args.size());
+
+ // Future taps should be re-enabled
+ ASSERT_FALSE(mReader->getContext()->isPreventingTouchpadTaps());
+
+ // taps before the threshold should still be ignored
+ currentTime += TAP_ENABLE_DELAY_NANOS.count();
+ flingGesture = Gesture(kGestureFling, currentTime, currentTime, /* vx= */ 0,
+ /* vy= */ 0, GESTURES_FLING_TAP_DOWN);
+ args = converter.handleGesture(currentTime, currentTime, currentTime, flingGesture);
+
+ ASSERT_EQ(1u, args.size());
+ ASSERT_THAT(std::get<NotifyMotionArgs>(args.front()),
+ AllOf(WithMotionAction(AMOTION_EVENT_ACTION_HOVER_MOVE), WithRelativeMotion(0, 0)));
+
+ tapGesture = Gesture(kGestureButtonsChange, currentTime, currentTime,
+ /* down= */ GESTURES_BUTTON_LEFT,
+ /* up= */ GESTURES_BUTTON_LEFT, /* is_tap= */ true);
+ args = converter.handleGesture(currentTime, currentTime, currentTime, tapGesture);
+
+ // no events should be generated
+ ASSERT_EQ(0u, args.size());
+
+ // taps after the threshold should be recognised
+ currentTime += 1;
+ flingGesture = Gesture(kGestureFling, currentTime, currentTime, /* vx= */ 0,
+ /* vy= */ 0, GESTURES_FLING_TAP_DOWN);
+ args = converter.handleGesture(currentTime, currentTime, currentTime, flingGesture);
+
+ ASSERT_EQ(1u, args.size());
+ ASSERT_THAT(std::get<NotifyMotionArgs>(args.front()),
+ AllOf(WithMotionAction(AMOTION_EVENT_ACTION_HOVER_MOVE), WithRelativeMotion(0, 0)));
+
+ tapGesture = Gesture(kGestureButtonsChange, currentTime, currentTime,
+ /* down= */ GESTURES_BUTTON_LEFT,
+ /* up= */ GESTURES_BUTTON_LEFT, /* is_tap= */ true);
+ args = converter.handleGesture(currentTime, currentTime, currentTime, tapGesture);
+
+ ASSERT_EQ(5u, args.size());
+ ASSERT_THAT(std::get<NotifyMotionArgs>(args.front()),
+ AllOf(WithMotionAction(AMOTION_EVENT_ACTION_DOWN), WithRelativeMotion(0.f, 0.f),
+ WithButtonState(AMOTION_EVENT_BUTTON_PRIMARY)));
+ args.pop_front();
+ ASSERT_THAT(std::get<NotifyMotionArgs>(args.front()),
+ AllOf(WithMotionAction(AMOTION_EVENT_ACTION_BUTTON_PRESS),
+ WithActionButton(AMOTION_EVENT_BUTTON_PRIMARY), WithButtonState(1),
+ WithRelativeMotion(0.f, 0.f)));
+ args.pop_front();
+ ASSERT_THAT(std::get<NotifyMotionArgs>(args.front()),
+ AllOf(WithMotionAction(AMOTION_EVENT_ACTION_BUTTON_RELEASE),
+ WithActionButton(AMOTION_EVENT_BUTTON_PRIMARY), WithButtonState(0),
+ WithRelativeMotion(0.f, 0.f)));
+ args.pop_front();
+ ASSERT_THAT(std::get<NotifyMotionArgs>(args.front()),
+ AllOf(WithMotionAction(AMOTION_EVENT_ACTION_UP), WithRelativeMotion(0.f, 0.f),
+ WithButtonState(0)));
+ args.pop_front();
+ ASSERT_THAT(std::get<NotifyMotionArgs>(args.front()),
+ AllOf(WithMotionAction(AMOTION_EVENT_ACTION_HOVER_MOVE), WithRelativeMotion(0, 0),
+ WithButtonState(0)));
+}
+
TEST_F_WITH_FLAGS(GestureConverterTest, ClickWithTapToClickDisabled,
REQUIRES_FLAGS_ENABLED(TOUCHPAD_PALM_REJECTION)) {
// Click should still produce button press/release events
@@ -2448,7 +2541,10 @@
}
TEST_F_WITH_FLAGS(GestureConverterTestWithChoreographer, TapWithTapToClickDisabled,
- REQUIRES_FLAGS_ENABLED(TOUCHPAD_PALM_REJECTION)) {
+ REQUIRES_FLAGS_ENABLED(TOUCHPAD_PALM_REJECTION),
+ REQUIRES_FLAGS_DISABLED(TOUCHPAD_PALM_REJECTION_V2)) {
+ nsecs_t currentTime = ARBITRARY_GESTURE_TIME;
+
// Tap should be ignored when disabled
mReader->getContext()->setPreventingTouchpadTaps(true);
@@ -2456,22 +2552,21 @@
GestureConverter converter(*mReader->getContext(), deviceContext, DEVICE_ID);
converter.setDisplayId(ADISPLAY_ID_DEFAULT);
- Gesture flingGesture(kGestureFling, ARBITRARY_GESTURE_TIME, ARBITRARY_GESTURE_TIME, /* vx= */ 0,
+ Gesture flingGesture(kGestureFling, currentTime, currentTime, /* vx= */ 0,
/* vy= */ 0, GESTURES_FLING_TAP_DOWN);
std::list<NotifyArgs> args =
- converter.handleGesture(ARBITRARY_TIME, READ_TIME, ARBITRARY_TIME, flingGesture);
+ converter.handleGesture(currentTime, currentTime, currentTime, flingGesture);
ASSERT_EQ(1u, args.size());
ASSERT_THAT(std::get<NotifyMotionArgs>(args.front()),
AllOf(WithMotionAction(AMOTION_EVENT_ACTION_HOVER_MOVE), WithCoords(0, 0),
WithRelativeMotion(0, 0), WithToolType(ToolType::FINGER), WithButtonState(0),
WithPressure(0.0f), WithDisplayId(ADISPLAY_ID_DEFAULT)));
- args.pop_front();
- Gesture tapGesture(kGestureButtonsChange, ARBITRARY_GESTURE_TIME, ARBITRARY_GESTURE_TIME,
+ Gesture tapGesture(kGestureButtonsChange, currentTime, currentTime,
/* down= */ GESTURES_BUTTON_LEFT,
/* up= */ GESTURES_BUTTON_LEFT, /* is_tap= */ true);
- args = converter.handleGesture(ARBITRARY_TIME, READ_TIME, ARBITRARY_TIME, tapGesture);
+ args = converter.handleGesture(currentTime, currentTime, currentTime, tapGesture);
// no events should be generated
ASSERT_EQ(0u, args.size());
@@ -2480,6 +2575,96 @@
ASSERT_FALSE(mReader->getContext()->isPreventingTouchpadTaps());
}
+TEST_F_WITH_FLAGS(GestureConverterTestWithChoreographer, TapWithTapToClickDisabledWithDelay,
+ REQUIRES_FLAGS_ENABLED(TOUCHPAD_PALM_REJECTION_V2)) {
+ nsecs_t currentTime = ARBITRARY_GESTURE_TIME;
+
+ // Tap should be ignored when disabled
+ mReader->getContext()->setPreventingTouchpadTaps(true);
+
+ InputDeviceContext deviceContext(*mDevice, EVENTHUB_ID);
+ GestureConverter converter(*mReader->getContext(), deviceContext, DEVICE_ID);
+ converter.setDisplayId(ADISPLAY_ID_DEFAULT);
+
+ Gesture flingGesture(kGestureFling, currentTime, currentTime, /* vx= */ 0,
+ /* vy= */ 0, GESTURES_FLING_TAP_DOWN);
+ std::list<NotifyArgs> args =
+ converter.handleGesture(currentTime, currentTime, currentTime, flingGesture);
+
+ ASSERT_EQ(1u, args.size());
+ ASSERT_THAT(std::get<NotifyMotionArgs>(args.front()),
+ AllOf(WithMotionAction(AMOTION_EVENT_ACTION_HOVER_MOVE), WithCoords(0, 0),
+ WithRelativeMotion(0, 0), WithToolType(ToolType::FINGER), WithButtonState(0),
+ WithPressure(0.0f), WithDisplayId(ADISPLAY_ID_DEFAULT)));
+
+ Gesture tapGesture(kGestureButtonsChange, currentTime, currentTime,
+ /* down= */ GESTURES_BUTTON_LEFT,
+ /* up= */ GESTURES_BUTTON_LEFT, /* is_tap= */ true);
+ args = converter.handleGesture(currentTime, currentTime, currentTime, tapGesture);
+
+ // no events should be generated
+ ASSERT_EQ(0u, args.size());
+
+ // Future taps should be re-enabled
+ ASSERT_FALSE(mReader->getContext()->isPreventingTouchpadTaps());
+
+ // taps before the threshold should still be ignored
+ currentTime += TAP_ENABLE_DELAY_NANOS.count();
+ flingGesture = Gesture(kGestureFling, currentTime, currentTime, /* vx= */ 0,
+ /* vy= */ 0, GESTURES_FLING_TAP_DOWN);
+ args = converter.handleGesture(currentTime, currentTime, currentTime, flingGesture);
+
+ ASSERT_EQ(1u, args.size());
+ ASSERT_THAT(std::get<NotifyMotionArgs>(args.front()),
+ AllOf(WithMotionAction(AMOTION_EVENT_ACTION_HOVER_MOVE), WithRelativeMotion(0, 0)));
+
+ tapGesture = Gesture(kGestureButtonsChange, currentTime, currentTime,
+ /* down= */ GESTURES_BUTTON_LEFT,
+ /* up= */ GESTURES_BUTTON_LEFT, /* is_tap= */ true);
+ args = converter.handleGesture(currentTime, currentTime, currentTime, tapGesture);
+
+ // no events should be generated
+ ASSERT_EQ(0u, args.size());
+
+ // taps after the threshold should be recognised
+ currentTime += 1;
+ flingGesture = Gesture(kGestureFling, currentTime, currentTime, /* vx= */ 0,
+ /* vy= */ 0, GESTURES_FLING_TAP_DOWN);
+ args = converter.handleGesture(currentTime, currentTime, currentTime, flingGesture);
+
+ ASSERT_EQ(1u, args.size());
+ ASSERT_THAT(std::get<NotifyMotionArgs>(args.front()),
+ AllOf(WithMotionAction(AMOTION_EVENT_ACTION_HOVER_MOVE), WithRelativeMotion(0, 0)));
+
+ tapGesture = Gesture(kGestureButtonsChange, currentTime, currentTime,
+ /* down= */ GESTURES_BUTTON_LEFT,
+ /* up= */ GESTURES_BUTTON_LEFT, /* is_tap= */ true);
+ args = converter.handleGesture(currentTime, currentTime, currentTime, tapGesture);
+
+ ASSERT_EQ(5u, args.size());
+ ASSERT_THAT(std::get<NotifyMotionArgs>(args.front()),
+ AllOf(WithMotionAction(AMOTION_EVENT_ACTION_DOWN), WithRelativeMotion(0.f, 0.f),
+ WithButtonState(AMOTION_EVENT_BUTTON_PRIMARY)));
+ args.pop_front();
+ ASSERT_THAT(std::get<NotifyMotionArgs>(args.front()),
+ AllOf(WithMotionAction(AMOTION_EVENT_ACTION_BUTTON_PRESS),
+ WithActionButton(AMOTION_EVENT_BUTTON_PRIMARY), WithButtonState(1),
+ WithRelativeMotion(0.f, 0.f)));
+ args.pop_front();
+ ASSERT_THAT(std::get<NotifyMotionArgs>(args.front()),
+ AllOf(WithMotionAction(AMOTION_EVENT_ACTION_BUTTON_RELEASE),
+ WithActionButton(AMOTION_EVENT_BUTTON_PRIMARY), WithButtonState(0),
+ WithRelativeMotion(0.f, 0.f)));
+ args.pop_front();
+ ASSERT_THAT(std::get<NotifyMotionArgs>(args.front()),
+ AllOf(WithMotionAction(AMOTION_EVENT_ACTION_UP), WithRelativeMotion(0.f, 0.f),
+ WithButtonState(0)));
+ args.pop_front();
+ ASSERT_THAT(std::get<NotifyMotionArgs>(args.front()),
+ AllOf(WithMotionAction(AMOTION_EVENT_ACTION_HOVER_MOVE), WithRelativeMotion(0, 0),
+ WithButtonState(0)));
+}
+
TEST_F_WITH_FLAGS(GestureConverterTestWithChoreographer, ClickWithTapToClickDisabled,
REQUIRES_FLAGS_ENABLED(TOUCHPAD_PALM_REJECTION)) {
// Click should still produce button press/release events