Override VelocityTracker strategy for non-differential axes only
VelocityTracker may use different strategies, depending on the system
flags. By default, the behaviour is like this:
AXIS_X -> lsq2
AXIS_Y -> lsq2
AXIS_SCROLL -> impulse
However, if we provide a specific strategy to VT, then things would
change. The new map would look like this:
AXIS_X -> provided strategy
AXIS_Y -> provided strategy
AXIS_SCROLL -> provided strategy
This works fine if the user specifies "impulse" as the desired strategy,
because impulse supports all of the axes.
However, lsq2 only works in non-differential mode.
The combination of "AXIS_SCROLL -> lsq2" is not allowed.
To fix this, we only allow the specified strategy to affect
non-differential axes.
This is fine, because currently, impulse is the only strategy that can
work in the differential mode.
To reproduce the issue:
1. Run `atest VelocityTrackerTest`. Test should pass, because the
default strategy is being used.
2. Connect the device to internet
3. Reboot the device
4. Run `atest VelocityTrackerTest` again. Test should fail because the
device would pick up an "lsq2" strategy value and then try to run the
axis_scroll tests with it.
Bug: 295290374
Test: atest VelocityTrackerTest
Change-Id: I702a2a3e58db3ce2e0ff0c33122839a527eebab2
Merged-In: I702a2a3e58db3ce2e0ff0c33122839a527eebab2
(cherry picked from commit 8a2e589e28329fd597dbb0d14f4f8f443fe9a4b0)
diff --git a/include/input/VelocityTracker.h b/include/input/VelocityTracker.h
index da97c3e..4257cb5 100644
--- a/include/input/VelocityTracker.h
+++ b/include/input/VelocityTracker.h
@@ -45,6 +45,7 @@
INT2 = 8,
LEGACY = 9,
MAX = LEGACY,
+ ftl_last = LEGACY,
};
struct Estimator {
@@ -95,8 +96,6 @@
// TODO(b/32830165): support axis-specific strategies.
VelocityTracker(const Strategy strategy = Strategy::DEFAULT);
- ~VelocityTracker();
-
/** Return true if the axis is supported for velocity tracking, false otherwise. */
static bool isAxisSupported(int32_t axis);
diff --git a/libs/input/VelocityTracker.cpp b/libs/input/VelocityTracker.cpp
index 8551e5f..078109a 100644
--- a/libs/input/VelocityTracker.cpp
+++ b/libs/input/VelocityTracker.cpp
@@ -16,7 +16,9 @@
#define LOG_TAG "VelocityTracker"
+#include <android-base/logging.h>
#include <array>
+#include <ftl/enum.h>
#include <inttypes.h>
#include <limits.h>
#include <math.h>
@@ -145,27 +147,19 @@
VelocityTracker::VelocityTracker(const Strategy strategy)
: mLastEventTime(0), mCurrentPointerIdBits(0), mOverrideStrategy(strategy) {}
-VelocityTracker::~VelocityTracker() {
-}
-
bool VelocityTracker::isAxisSupported(int32_t axis) {
return DEFAULT_STRATEGY_BY_AXIS.find(axis) != DEFAULT_STRATEGY_BY_AXIS.end();
}
void VelocityTracker::configureStrategy(int32_t axis) {
const bool isDifferentialAxis = DIFFERENTIAL_AXES.find(axis) != DIFFERENTIAL_AXES.end();
-
- std::unique_ptr<VelocityTrackerStrategy> createdStrategy;
- if (mOverrideStrategy != VelocityTracker::Strategy::DEFAULT) {
- createdStrategy = createStrategy(mOverrideStrategy, /*deltaValues=*/isDifferentialAxis);
+ if (isDifferentialAxis || mOverrideStrategy == VelocityTracker::Strategy::DEFAULT) {
+ // Do not allow overrides of strategies for differential axes, for now.
+ mConfiguredStrategies[axis] = createStrategy(DEFAULT_STRATEGY_BY_AXIS.at(axis),
+ /*deltaValues=*/isDifferentialAxis);
} else {
- createdStrategy = createStrategy(DEFAULT_STRATEGY_BY_AXIS.at(axis),
- /*deltaValues=*/isDifferentialAxis);
+ mConfiguredStrategies[axis] = createStrategy(mOverrideStrategy, /*deltaValues=*/false);
}
-
- LOG_ALWAYS_FATAL_IF(createdStrategy == nullptr,
- "Could not create velocity tracker strategy for axis '%" PRId32 "'!", axis);
- mConfiguredStrategies[axis] = std::move(createdStrategy);
}
std::unique_ptr<VelocityTrackerStrategy> VelocityTracker::createStrategy(
@@ -213,6 +207,9 @@
default:
break;
}
+ LOG(FATAL) << "Invalid strategy: " << ftl::enum_string(strategy)
+ << ", deltaValues = " << deltaValues;
+
return nullptr;
}
diff --git a/libs/input/tests/VelocityTracker_test.cpp b/libs/input/tests/VelocityTracker_test.cpp
index ae72109..73f25cc 100644
--- a/libs/input/tests/VelocityTracker_test.cpp
+++ b/libs/input/tests/VelocityTracker_test.cpp
@@ -282,6 +282,11 @@
const std::vector<std::pair<std::chrono::nanoseconds, float>>& motions,
std::optional<float> targetVelocity) {
checkVelocity(computeVelocity(strategy, motions, AMOTION_EVENT_AXIS_SCROLL), targetVelocity);
+ // The strategy LSQ2 is not compatible with AXIS_SCROLL. In those situations, we should fall
+ // back to a strategy that supports differential axes.
+ checkVelocity(computeVelocity(VelocityTracker::Strategy::LSQ2, motions,
+ AMOTION_EVENT_AXIS_SCROLL),
+ targetVelocity);
}
static void computeAndCheckQuadraticEstimate(const std::vector<PlanarMotionEventEntry>& motions,
