diff options
author | Harry Cutts <hcutts@chromium.org> | 2023-03-08 15:15:33 +0000 |
---|---|---|
committer | Chromeos LUCI <chromeos-scoped@luci-project-accounts.iam.gserviceaccount.com> | 2023-03-09 18:36:38 +0000 |
commit | 2a80cba49fb09f6f2632b61f21ea7dc62acd82d7 (patch) | |
tree | 63fbfbe017728b876aae982f54b2df5fe5508393 | |
parent | 1273082d2427e36db79a1f9cc07a6db199a8c186 (diff) | |
download | libchrome-gestures-2a80cba49fb09f6f2632b61f21ea7dc62acd82d7.tar.gz |
scaling: Default to 32 units/mm if X/Y resolution isn't specified
Previously it would just be 0, causing the scales to be infinity and any
touch positions output from ScalingFilterInterpreter to be NaN. Looking
at the resolutions reported by various Chromebook touchpads, 32 seems to
be a reasonable default, especially for the sort of low-end touchpad
that wouldn't specify a resolution.
BUG=b:246587538
TEST=(On Android) Connect a Sony DualShock 4 or DualSense controller,
confirm that the cursor etc. now responds to gestures; run unit
tests
Change-Id: I50e516d17948aa6e0bbd6ec7532048b7345d223c
Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/platform/gestures/+/4324037
Commit-Queue: Harry Cutts <hcutts@chromium.org>
Tested-by: Harry Cutts <hcutts@chromium.org>
Code-Coverage: Sean O'Brien <seobrien@chromium.org>
Reviewed-by: Sean O'Brien <seobrien@chromium.org>
-rw-r--r-- | include/gestures.h | 3 | ||||
-rw-r--r-- | src/scaling_filter_interpreter.cc | 9 | ||||
-rw-r--r-- | src/scaling_filter_interpreter_unittest.cc | 43 |
3 files changed, 51 insertions, 4 deletions
diff --git a/include/gestures.h b/include/gestures.h index e4f7cc2..f21d531 100644 --- a/include/gestures.h +++ b/include/gestures.h @@ -55,7 +55,8 @@ struct HardwareProperties { float right; // The maximum Y coordinate that the device can report. float bottom; - // The resolutions of the X and Y axes, in units per mm. + // The resolutions of the X and Y axes, in units per mm. Set to 0 if the + // resolution is unknown. float res_x; float res_y; diff --git a/src/scaling_filter_interpreter.cc b/src/scaling_filter_interpreter.cc index 1094455..2d2aad9 100644 --- a/src/scaling_filter_interpreter.cc +++ b/src/scaling_filter_interpreter.cc @@ -318,8 +318,13 @@ void ScalingFilterInterpreter::Initialize(const HardwareProperties* hwprops, Metrics* metrics, MetricsProperties* mprops, GestureConsumer* consumer) { - tp_x_scale_ = 1.0 / hwprops->res_x; - tp_y_scale_ = 1.0 / hwprops->res_y; + // If the touchpad doesn't specify its resolution in one or both axes, use a + // reasonable default instead. + float res_x = hwprops->res_x != 0 ? hwprops->res_x : 32; + float res_y = hwprops->res_y != 0 ? hwprops->res_y : 32; + + tp_x_scale_ = 1.0 / res_x; + tp_y_scale_ = 1.0 / res_y; tp_x_translate_ = -1.0 * (hwprops->left * tp_x_scale_); tp_y_translate_ = -1.0 * (hwprops->top * tp_y_scale_); diff --git a/src/scaling_filter_interpreter_unittest.cc b/src/scaling_filter_interpreter_unittest.cc index d701c7f..93f8e79 100644 --- a/src/scaling_filter_interpreter_unittest.cc +++ b/src/scaling_filter_interpreter_unittest.cc @@ -68,7 +68,7 @@ class ScalingFilterInterpreterTestInterpreter : public Interpreter { EXPECT_FLOAT_EQ(expected_pressures_.front(), hwstate->fingers[0].pressure); expected_pressures_.pop_front(); - } else { + } else if (!expected_finger_cnt_.empty() && !expected_touch_cnt_.empty()) { // Test if the low pressure event is dropped EXPECT_EQ(expected_finger_cnt_.front(), hwstate->finger_cnt); expected_finger_cnt_.pop_front(); @@ -272,6 +272,47 @@ TEST(ScalingFilterInterpreterTest, SimpleTest) { out = wrapper.SyncInterpret(&hs2[2], NULL); } +TEST(ScalingFilterInterpreterTest, ResolutionFallback) { + ScalingFilterInterpreterTestInterpreter* base_interpreter = + new ScalingFilterInterpreterTestInterpreter; + ScalingFilterInterpreter interpreter(NULL, base_interpreter, NULL, + GESTURES_DEVCLASS_TOUCHPAD); + HardwareProperties initial_hwprops = { + 0, 0, 2000, 1000, // left, top, right, bottom + 0, 0, // X/Y resolutions (pixels/mm) + 0, 0, // screen DPI X, Y (deprecated) + -1, // orientation minimum + 2, // orientation maximum + 2, 5, // max fingers, max_touch + 0, 0, 0, // t5r2, semi, button pad + 0, 0, // has wheel, vertical wheel is high resolution + 0, // haptic pad + }; + HardwareProperties expected_hwprops = { + 0, 0, 2000 / 32.0, 1000 / 32.0, // left, top, right, bottom + 1, 1, // X/Y resolutions (pixels/mm) + 25.4, 25.4, // x DPI, y DPI + -M_PI_4, // orientation minimum (1 tick above X-axis) + M_PI_2, // orientation maximum + 2, 5, 0, 0, 0, // max_fingers, max_touch, t5r2, semi_mt, is button pad + 0, 0, // has wheel, vertical wheel is high resolution + 0, // is haptic pad + }; + base_interpreter->expected_hwprops_ = expected_hwprops; + + TestInterpreterWrapper wrapper(&interpreter, &initial_hwprops); + EXPECT_TRUE(base_interpreter->initialize_called_); + + FingerState fs = { 1, 0, 0, 0, 1, 0, 1000, 500, 1, 0 }; + HardwareState hs = make_hwstate(10000, 0, 1, 1, &fs); + + base_interpreter->expected_coordinates_.push_back( + std::vector<pair<float, float>>(1, make_pair( + static_cast<float>(1000 / 32.0), static_cast<float>(500 / 32.0)))); + + wrapper.SyncInterpret(&hs, NULL); +} + static void RunTouchMajorAndMinorTest( ScalingFilterInterpreterTestInterpreter* base_interpreter, ScalingFilterInterpreter* interpreter, |