ImmediateInterpreter: Handle right taps that exhibit some "light" merging
We noticed on some hardware that when two fingers join or leave,
sometimes there is a frame of data that has only one contact, toward
the center of where the two are in other frames, and this contact has
very light pressure. This CL uses that very light pressure as a guage
that these frames shouldn't count toward moving a lot, since such
movement would cause a tap to look like it's not a tap.
BUG=chromium-os:30097
TEST=unittest, which is based on logs that failed, but now pass
Change-Id: Ib727842f22bb7f4c7c0ada8f9f6d09f1f513d7c4
Reviewed-on: https://gerrit.chromium.org/gerrit/21397
Reviewed-by: Daniel Kurtz <djkurtz@chromium.org>
Tested-by: Andrew de los Reyes <adlr@chromium.org>
Commit-Ready: Andrew de los Reyes <adlr@chromium.org>
diff --git a/include/immediate_interpreter.h b/include/immediate_interpreter.h
index 1e6adaa..92824c5 100644
--- a/include/immediate_interpreter.h
+++ b/include/immediate_interpreter.h
@@ -52,6 +52,8 @@
void NoteRelease(short the_id); // Adds to released_
void Remove(short the_id); // Removes from touched_ and released_
+ float CotapMinPressure() const;
+
map<short, FingerState, kMaxTapFingers> touched_;
set<short, kMaxTapFingers> released_;
// At least one finger must meet the minimum pressure requirement during a
@@ -105,6 +107,7 @@
FRIEND_TEST(ImmediateInterpreterTest, TapRecordTest);
FRIEND_TEST(ImmediateInterpreterTest, TapToClickEnableTest);
FRIEND_TEST(ImmediateInterpreterTest, TapToClickKeyboardTest);
+ FRIEND_TEST(ImmediateInterpreterTest, TapToClickLowPressureBeginOrEndTest);
FRIEND_TEST(ImmediateInterpreterTest, TapToClickStateMachineTest);
FRIEND_TEST(ImmediateInterpreterTest, ThumbRetainReevaluateTest);
FRIEND_TEST(ImmediateInterpreterTest, ThumbRetainTest);
diff --git a/src/immediate_interpreter.cc b/src/immediate_interpreter.cc
index a60de28..8caf96a 100644
--- a/src/immediate_interpreter.cc
+++ b/src/immediate_interpreter.cc
@@ -72,6 +72,10 @@
released_.erase(the_id);
}
+float TapRecord::CotapMinPressure() const {
+ return immediate_interpreter_->tap_min_pressure() * 0.5;
+}
+
void TapRecord::Update(const HardwareState& hwstate,
const HardwareState& prev_hwstate,
const set<short, kMaxTapFingers>& added,
@@ -110,8 +114,7 @@
for_each(removed.begin(), removed.end(),
bind1st(mem_fun(&TapRecord::NoteRelease), this));
// Check if min tap/cotap pressure met yet
- const float cotap_min_pressure =
- immediate_interpreter_->tap_min_pressure() * 0.5;
+ const float cotap_min_pressure = CotapMinPressure();
for (map<short, FingerState, kMaxTapFingers>::iterator it =
touched_.begin(), e = touched_.end();
it != e; ++it) {
@@ -119,8 +122,14 @@
if (fs) {
if (fs->pressure >= immediate_interpreter_->tap_min_pressure())
min_tap_pressure_met_.insert(fs->tracking_id);
- if (fs->pressure >= cotap_min_pressure)
+ if (fs->pressure >= cotap_min_pressure) {
min_cotap_pressure_met_.insert(fs->tracking_id);
+ if ((*it).second.pressure < cotap_min_pressure) {
+ // Update existing record, since the old one hadn't met the cotap
+ // pressure
+ (*it).second = *fs;
+ }
+ }
}
}
Log("Done Updating TapRecord.");
@@ -138,11 +147,17 @@
bool TapRecord::Moving(const HardwareState& hwstate,
const float dist_max) const {
+ const float cotap_min_pressure = CotapMinPressure();
for (map<short, FingerState, kMaxTapFingers>::const_iterator it =
touched_.begin(), e = touched_.end(); it != e; ++it) {
const FingerState* fs = hwstate.GetFingerState((*it).first);
if (!fs)
continue;
+ // Only look for moving when current frame meets cotap pressure and
+ // our history contains a contact that's met cotap pressure.
+ if (fs->pressure < cotap_min_pressure ||
+ (*it).second.pressure < cotap_min_pressure)
+ continue;
// Compute distance moved
float dist_x = fs->position_x - (*it).second.position_x;
float dist_y = fs->position_y - (*it).second.position_y;
diff --git a/src/immediate_interpreter_unittest.cc b/src/immediate_interpreter_unittest.cc
index 3c4bc1f..afb94a9 100644
--- a/src/immediate_interpreter_unittest.cc
+++ b/src/immediate_interpreter_unittest.cc
@@ -1894,6 +1894,101 @@
}
}
+namespace {
+
+struct TapToClickLowPressureBeginOrEndInputs {
+ stime_t now;
+ float x0, y0, p0, id0, x1, y1, p1, id1; // (x, y), pressure, tracking id
+};
+
+} // namespace {}
+
+// Test that if a tap contact has some frames before and after that tap, with
+// a finger that's located far from the tap spot, but has low pressure at that
+// location, it's still a tap. We see this happen on some hardware particularly
+// for right clicks. This is based on two logs from Ken Moore.
+TEST(ImmediateInterpreterTest, TapToClickLowPressureBeginOrEndTest) {
+ scoped_ptr<ImmediateInterpreter> ii;
+ HardwareProperties hwprops = {
+ 0, // left edge
+ 0, // top edge
+ 106.666672, // right edge
+ 68.000000, // bottom edge
+ 1, // pixels/TP width
+ 1, // pixels/TP height
+ 25.4, // screen DPI x
+ 25.4, // screen DPI y
+ 15, // max fingers
+ 5, // max touch
+ 0, // t5r2
+ 0, // semi-mt
+ true // is button pad
+ };
+
+ TapToClickLowPressureBeginOrEndInputs inputs[] = {
+ // Two runs
+ { 32.4901, 55.5, 24.8, 7.0, 1, 0.0, 0.0, 0.0, -1 },
+ { 32.5010, 57.7, 25.0, 43.0, 1, 42.0, 27.5, 36.0, 2 },
+ { 32.5118, 58.0, 25.0, 44.0, 1, 42.0, 27.5, 43.0, 2 },
+ { 32.5227, 58.0, 25.0, 44.0, 1, 42.0, 27.6, 44.0, 2 },
+ { 32.5335, 58.0, 25.0, 45.0, 1, 42.0, 27.6, 45.0, 2 },
+ { 32.5443, 58.0, 25.0, 44.0, 1, 42.0, 27.6, 45.0, 2 },
+ { 32.5552, 58.0, 25.0, 44.0, 1, 42.0, 27.6, 44.0, 2 },
+ { 32.5661, 58.0, 25.0, 42.0, 1, 42.0, 27.6, 42.0, 2 },
+ { 32.5769, 58.0, 25.0, 35.0, 1, 42.0, 27.6, 35.0, 2 },
+ { 32.5878, 58.0, 25.0, 15.0, 1, 41.9, 27.6, 18.0, 2 },
+ { 32.5965, 45.9, 27.5, 7.0, 2, 0.0, 0.0, 0.0, -1 },
+ { 32.6042, 0.0, 0.0, 0.0, -1, 0.0, 0.0, 0.0, -1 },
+
+ { 90.6057, 64.0, 37.0, 18.0, 1, 0.0, 0.0, 0.0, -1 },
+ { 90.6144, 63.6, 37.0, 43.0, 1, 0.0, 0.0, 0.0, -1 },
+ { 90.6254, 63.6, 37.0, 43.0, 1, 46.5, 40.2, 47.0, 2 },
+ { 90.6361, 63.6, 37.0, 44.0, 1, 46.5, 40.2, 44.0, 2 },
+ { 90.6470, 63.6, 37.0, 45.0, 1, 46.5, 40.2, 46.0, 2 },
+ { 90.6579, 63.6, 37.0, 45.0, 1, 46.5, 40.2, 46.0, 2 },
+ { 90.6686, 63.6, 37.0, 45.0, 1, 46.5, 40.2, 47.0, 2 },
+ { 90.6795, 63.6, 37.0, 46.0, 1, 46.5, 40.2, 47.0, 2 },
+ { 90.6903, 63.6, 37.0, 45.0, 1, 46.5, 40.2, 46.0, 2 },
+ { 90.7012, 63.6, 37.0, 44.0, 1, 46.3, 40.2, 44.0, 2 },
+ { 90.7121, 63.6, 37.2, 38.0, 1, 46.4, 40.2, 31.0, 2 },
+ { 90.7229, 63.6, 37.4, 22.0, 1, 46.4, 40.2, 12.0, 2 },
+ { 90.7317, 61.1, 38.0, 8.0, 1, 0.0, 0.0, 0.0, -1 },
+ { 90.7391, 0.0, 0.0, 0.0, -1, 0.0, 0.0, 0.0, -1 },
+ };
+
+ bool reset_next_time = true;
+ for (size_t i = 0; i < arraysize(inputs); i++) {
+ const TapToClickLowPressureBeginOrEndInputs& input = inputs[i];
+ if (reset_next_time) {
+ ii.reset(new ImmediateInterpreter(NULL));
+ ii->SetHardwareProperties(hwprops);
+ ii->tap_enable_.val_ = 1;
+ reset_next_time = false;
+ }
+ // Prep inputs
+ FingerState fs[] = {
+ { 0, 0, 0, 0, input.p0, 0, input.x0, input.y0, input.id0, 0 },
+ { 0, 0, 0, 0, input.p1, 0, input.x1, input.y1, input.id1, 0 },
+ };
+ short finger_cnt = fs[0].tracking_id == -1 ? 0 :
+ (fs[1].tracking_id == -1 ? 1 : 2);
+ HardwareState hs = { input.now, 0, finger_cnt, finger_cnt, fs };
+ stime_t timeout = -1;
+ Gesture* gs = ii->SyncInterpret(&hs, &timeout);
+ if (finger_cnt > 0) {
+ // Expect no timeout
+ EXPECT_LT(timeout, 0);
+ } else {
+ // No timeout b/c it's right click. Expect the right click gesture
+ ASSERT_NE(static_cast<Gesture*>(NULL), gs) << "timeout:" << timeout;
+ EXPECT_EQ(kGestureTypeButtonsChange, gs->type);
+ EXPECT_EQ(GESTURES_BUTTON_RIGHT, gs->details.buttons.down);
+ EXPECT_EQ(GESTURES_BUTTON_RIGHT, gs->details.buttons.up);
+ reset_next_time = true; // All done w/ this run.
+ }
+ }
+}
+
// Does two tap gestures, one with keyboard interference.
TEST(ImmediateInterpreterTest, TapToClickKeyboardTest) {
scoped_ptr<ImmediateInterpreter> ii;
