From 9892a3000db1dc71aed6eb371dc5c7a0d8a42e22 Mon Sep 17 00:00:00 2001
From: Harry Cutts <hcutts@google.com>
Date: Wed, 22 Mar 2023 15:38:10 +0000
Subject: Fix touchpad palm filtering logic

When I added this logic [0], I was under the mistaken impression that
the Gesture library required touches that were leaving the pad to be
reported as FingerStates with a tracking ID of -1. It turns out that it
doesn't, and in fact such FingerStates shouldn't be sent to it [1]. This
makes the palm filtering logic simpler, since we can just always ignore
a slot if it has the palm tool type, rather than having to make one last
FingerState for it if was previously a finger.

[0]: Change ID I7d898889d4bf4e8068194d22150c02a69b59c435
[1]: https://crrev.com/c/4360158

Bug: 251196347
Bug: 270041770
Test: atest inputflinger_tests:HardwareStateConverterTest
Test: limited manual testing
Change-Id: I7f70f1b677ba19a472fd1670918a82dfe65c6f39
---
 .../mapper/gestures/HardwareStateConverter.cpp       | 20 ++++++--------------
 .../reader/mapper/gestures/HardwareStateConverter.h  |  1 -
 .../tests/HardwareStateConverter_test.cpp            |  3 +--
 3 files changed, 7 insertions(+), 17 deletions(-)

diff --git a/services/inputflinger/reader/mapper/gestures/HardwareStateConverter.cpp b/services/inputflinger/reader/mapper/gestures/HardwareStateConverter.cpp
index c091a51386..d344babe72 100644
--- a/services/inputflinger/reader/mapper/gestures/HardwareStateConverter.cpp
+++ b/services/inputflinger/reader/mapper/gestures/HardwareStateConverter.cpp
@@ -14,6 +14,9 @@
  * limitations under the License.
  */
 
+// clang-format off
+#include "../Macros.h"
+// clang-format on
 #include "gestures/HardwareStateConverter.h"
 
 #include <chrono>
@@ -80,14 +83,9 @@ SelfContainedHardwareState HardwareStateConverter::produceHardwareState(nsecs_t
     schs.fingers.clear();
     for (size_t i = 0; i < mMotionAccumulator.getSlotCount(); i++) {
         MultiTouchMotionAccumulator::Slot slot = mMotionAccumulator.getSlot(i);
-        if (!slot.isInUse()) {
-            continue;
-        }
         // Some touchpads continue to report contacts even after they've identified them as palms.
-        // We want to exclude these contacts from the HardwareStates, but still need to report a
-        // tracking ID of -1 if a finger turns into a palm.
-        const bool isPalm = slot.getToolType() == AMOTION_EVENT_TOOL_TYPE_PALM;
-        if (isPalm && mFingerSlots.find(i) == mFingerSlots.end()) {
+        // We want to exclude these contacts from the HardwareStates.
+        if (!slot.isInUse() || slot.getToolType() == AMOTION_EVENT_TOOL_TYPE_PALM) {
             continue;
         }
 
@@ -101,12 +99,7 @@ SelfContainedHardwareState HardwareStateConverter::produceHardwareState(nsecs_t
         fingerState.orientation = slot.getOrientation();
         fingerState.position_x = slot.getX();
         fingerState.position_y = slot.getY();
-        fingerState.tracking_id = isPalm ? -1 : slot.getTrackingId();
-        if (fingerState.tracking_id == -1) {
-            mFingerSlots.erase(i);
-        } else {
-            mFingerSlots.insert(i);
-        }
+        fingerState.tracking_id = slot.getTrackingId();
     }
     schs.state.fingers = schs.fingers.data();
     schs.state.finger_cnt = schs.fingers.size();
@@ -117,7 +110,6 @@ SelfContainedHardwareState HardwareStateConverter::produceHardwareState(nsecs_t
 void HardwareStateConverter::reset() {
     mCursorButtonAccumulator.reset(mDeviceContext);
     mTouchButtonAccumulator.reset();
-    mFingerSlots.clear();
     mMscTimestamp = 0;
 }
 
diff --git a/services/inputflinger/reader/mapper/gestures/HardwareStateConverter.h b/services/inputflinger/reader/mapper/gestures/HardwareStateConverter.h
index d6787b7e3f..c314b0d771 100644
--- a/services/inputflinger/reader/mapper/gestures/HardwareStateConverter.h
+++ b/services/inputflinger/reader/mapper/gestures/HardwareStateConverter.h
@@ -54,7 +54,6 @@ private:
     MultiTouchMotionAccumulator mMotionAccumulator;
     TouchButtonAccumulator mTouchButtonAccumulator;
     int32_t mMscTimestamp = 0;
-    std::set<size_t> mFingerSlots;
 };
 
 } // namespace android
diff --git a/services/inputflinger/tests/HardwareStateConverter_test.cpp b/services/inputflinger/tests/HardwareStateConverter_test.cpp
index 36b9bab14e..3e9724100c 100644
--- a/services/inputflinger/tests/HardwareStateConverter_test.cpp
+++ b/services/inputflinger/tests/HardwareStateConverter_test.cpp
@@ -231,8 +231,7 @@ TEST_F(HardwareStateConverterTest, OneFingerTurningIntoAPalm) {
 
     schs = processSync(conv, time);
     ASSERT_TRUE(schs.has_value());
-    ASSERT_EQ(1, schs->state.finger_cnt);
-    EXPECT_EQ(-1, schs->state.fingers[0].tracking_id);
+    ASSERT_EQ(0, schs->state.finger_cnt);
 
     processAxis(conv, time, EV_ABS, ABS_MT_POSITION_X, 53);
     processAxis(conv, time, EV_ABS, ABS_MT_POSITION_Y, 97);
-- 
cgit v1.2.3-59-g8ed1b