Improve VelocityTracker Strategy Handling
The native VelocityTracker class has been made 1-dimensional to allow
support for axes beyond the historically-supported X/Y axes. This means
that a given VelocityTracker instance does not necessarily handle data
for all supported axes. As such, this CL sets up tracking strategy for
an axis only on the first occassion a data arrives for the axis.
Furthermore, to support use cases where different strategies may suit
different axes better, we have introduced per-axis default strategies.
Bug: 32830165
Test: atest libinput_tests; manual on-device fling tests
Change-Id: I3f7115fdcec78d1577e90e9a55175e8868bf2dfb
diff --git a/include/input/VelocityTracker.h b/include/input/VelocityTracker.h
index 53603c5..da4d877 100644
--- a/include/input/VelocityTracker.h
+++ b/include/input/VelocityTracker.h
@@ -146,16 +146,12 @@
inline int32_t getActivePointerId() const { return mActivePointerId; }
private:
- // The default velocity tracker strategy.
+ // All axes supported for velocity tracking, mapped to their default strategies.
// Although other strategies are available for testing and comparison purposes,
- // this is the strategy that applications will actually use. Be very careful
+ // the default strategy is the one that applications will actually use. Be very careful
// when adjusting the default strategy because it can dramatically affect
// (often in a bad way) the user experience.
- // TODO(b/32830165): define default strategy per axis.
- static const Strategy DEFAULT_STRATEGY = Strategy::LSQ2;
-
- // Set of all axes supported for velocity tracking.
- static const std::set<int32_t> SUPPORTED_AXES;
+ static const std::map<int32_t, Strategy> DEFAULT_STRATEGY_BY_AXIS;
// Axes specifying location on a 2D plane (i.e. X and Y).
static const std::set<int32_t> PLANAR_AXES;
@@ -163,9 +159,17 @@
nsecs_t mLastEventTime;
BitSet32 mCurrentPointerIdBits;
int32_t mActivePointerId;
- std::map<int32_t /*axis*/, std::unique_ptr<VelocityTrackerStrategy>> mStrategies;
- void configureStrategy(int32_t axis, const Strategy strategy);
+ // An override strategy passed in the constructor to be used for all axes.
+ // This strategy will apply to all axes, unless the default strategy is specified here.
+ // When default strategy is specified, then each axis will use a potentially different strategy
+ // based on a hardcoded mapping.
+ const Strategy mOverrideStrategy;
+ // Maps axes to their respective VelocityTrackerStrategy instances.
+ // Note that, only axes that have had MotionEvents (and not all supported axes) will be here.
+ std::map<int32_t /*axis*/, std::unique_ptr<VelocityTrackerStrategy>> mConfiguredStrategies;
+
+ void configureStrategy(int32_t axis);
static std::unique_ptr<VelocityTrackerStrategy> createStrategy(const Strategy strategy);
};
@@ -181,7 +185,6 @@
public:
virtual ~VelocityTrackerStrategy() { }
- virtual void clear() = 0;
virtual void clearPointers(BitSet32 idBits) = 0;
virtual void addMovement(nsecs_t eventTime, BitSet32 idBits,
const std::vector<float>& positions) = 0;
@@ -213,7 +216,6 @@
LeastSquaresVelocityTrackerStrategy(uint32_t degree, Weighting weighting = WEIGHTING_NONE);
virtual ~LeastSquaresVelocityTrackerStrategy();
- virtual void clear();
virtual void clearPointers(BitSet32 idBits);
void addMovement(nsecs_t eventTime, BitSet32 idBits,
const std::vector<float>& positions) override;
@@ -254,7 +256,6 @@
IntegratingVelocityTrackerStrategy(uint32_t degree);
~IntegratingVelocityTrackerStrategy();
- virtual void clear();
virtual void clearPointers(BitSet32 idBits);
void addMovement(nsecs_t eventTime, BitSet32 idBits,
const std::vector<float>& positions) override;
@@ -287,7 +288,6 @@
LegacyVelocityTrackerStrategy();
virtual ~LegacyVelocityTrackerStrategy();
- virtual void clear();
virtual void clearPointers(BitSet32 idBits);
void addMovement(nsecs_t eventTime, BitSet32 idBits,
const std::vector<float>& positions) override;
@@ -320,7 +320,6 @@
ImpulseVelocityTrackerStrategy();
virtual ~ImpulseVelocityTrackerStrategy();
- virtual void clear();
virtual void clearPointers(BitSet32 idBits);
void addMovement(nsecs_t eventTime, BitSet32 idBits,
const std::vector<float>& positions) override;
diff --git a/libs/input/VelocityTracker.cpp b/libs/input/VelocityTracker.cpp
index 6f88a38..527a75b 100644
--- a/libs/input/VelocityTracker.cpp
+++ b/libs/input/VelocityTracker.cpp
@@ -125,39 +125,32 @@
// --- VelocityTracker ---
-const std::set<int32_t> VelocityTracker::SUPPORTED_AXES = {AMOTION_EVENT_AXIS_X,
- AMOTION_EVENT_AXIS_Y};
+const std::map<int32_t, VelocityTracker::Strategy> VelocityTracker::DEFAULT_STRATEGY_BY_AXIS =
+ {{AMOTION_EVENT_AXIS_X, VelocityTracker::Strategy::LSQ2},
+ {AMOTION_EVENT_AXIS_Y, VelocityTracker::Strategy::LSQ2}};
const std::set<int32_t> VelocityTracker::PLANAR_AXES = {AMOTION_EVENT_AXIS_X, AMOTION_EVENT_AXIS_Y};
VelocityTracker::VelocityTracker(const Strategy strategy)
- : mLastEventTime(0), mCurrentPointerIdBits(0), mActivePointerId(-1) {
- // Configure the strategy for each axis.
- for (int32_t axis : SUPPORTED_AXES) {
- configureStrategy(axis, strategy);
- }
-}
+ : mLastEventTime(0),
+ mCurrentPointerIdBits(0),
+ mActivePointerId(-1),
+ mOverrideStrategy(strategy) {}
VelocityTracker::~VelocityTracker() {
}
-void VelocityTracker::configureStrategy(int32_t axis, const Strategy strategy) {
+void VelocityTracker::configureStrategy(int32_t axis) {
std::unique_ptr<VelocityTrackerStrategy> createdStrategy;
-
- if (strategy == VelocityTracker::Strategy::DEFAULT) {
- createdStrategy = createStrategy(VelocityTracker::DEFAULT_STRATEGY);
+ if (mOverrideStrategy != VelocityTracker::Strategy::DEFAULT) {
+ createdStrategy = createStrategy(mOverrideStrategy);
} else {
- createdStrategy = createStrategy(strategy);
+ createdStrategy = createStrategy(VelocityTracker::DEFAULT_STRATEGY_BY_AXIS.at(axis));
}
- if (createdStrategy == nullptr) {
- ALOGE("Unrecognized velocity tracker strategy %" PRId32 ".", strategy);
- createdStrategy = createStrategy(VelocityTracker::DEFAULT_STRATEGY);
- LOG_ALWAYS_FATAL_IF(createdStrategy == nullptr,
- "Could not create the default velocity tracker strategy '%" PRId32 "'!",
- strategy);
- }
- mStrategies[axis] = std::move(createdStrategy);
+ 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(
@@ -211,9 +204,7 @@
void VelocityTracker::clear() {
mCurrentPointerIdBits.clear();
mActivePointerId = -1;
- for (int32_t axis : SUPPORTED_AXES) {
- mStrategies[axis]->clear();
- }
+ mConfiguredStrategies.clear();
}
void VelocityTracker::clearPointers(BitSet32 idBits) {
@@ -224,8 +215,8 @@
mActivePointerId = !remainingIdBits.isEmpty() ? remainingIdBits.firstMarkedBit() : -1;
}
- for (int32_t axis : SUPPORTED_AXES) {
- mStrategies[axis]->clearPointers(idBits);
+ for (const auto& [_, strategy] : mConfiguredStrategies) {
+ strategy->clearPointers(idBits);
}
}
@@ -242,9 +233,7 @@
// We have not received any movements for too long. Assume that all pointers
// have stopped.
- for (const auto& [_, strategy] : mStrategies) {
- strategy->clear();
- }
+ mConfiguredStrategies.clear();
}
mLastEventTime = eventTime;
@@ -257,7 +246,10 @@
LOG_ALWAYS_FATAL_IF(idBits.count() != positionValues.size(),
"Mismatching number of pointers, idBits=%" PRIu32 ", positions=%zu",
idBits.count(), positionValues.size());
- mStrategies[axis]->addMovement(eventTime, idBits, positionValues);
+ if (mConfiguredStrategies.find(axis) == mConfiguredStrategies.end()) {
+ configureStrategy(axis);
+ }
+ mConfiguredStrategies[axis]->addMovement(eventTime, idBits, positionValues);
}
if (DEBUG_VELOCITY) {
@@ -323,7 +315,7 @@
// We have not received any movements for too long. Assume that all pointers
// have stopped.
for (int32_t axis : PLANAR_AXES) {
- mStrategies[axis]->clear();
+ mConfiguredStrategies.erase(axis);
}
}
// These actions because they do not convey any new information about
@@ -385,7 +377,7 @@
VelocityTracker::ComputedVelocity VelocityTracker::getComputedVelocity(int32_t units,
float maxVelocity) {
ComputedVelocity computedVelocity;
- for (int32_t axis : SUPPORTED_AXES) {
+ for (const auto& [axis, _] : mConfiguredStrategies) {
BitSet32 copyIdBits = BitSet32(mCurrentPointerIdBits);
while (!copyIdBits.isEmpty()) {
uint32_t id = copyIdBits.clearFirstMarkedBit();
@@ -401,28 +393,22 @@
}
bool VelocityTracker::getEstimator(int32_t axis, uint32_t id, Estimator* outEstimator) const {
- if (SUPPORTED_AXES.find(axis) == SUPPORTED_AXES.end()) {
+ const auto& it = mConfiguredStrategies.find(axis);
+ if (it == mConfiguredStrategies.end()) {
return false;
}
- return mStrategies.at(axis)->getEstimator(id, outEstimator);
+ return it->second->getEstimator(id, outEstimator);
}
// --- LeastSquaresVelocityTrackerStrategy ---
-LeastSquaresVelocityTrackerStrategy::LeastSquaresVelocityTrackerStrategy(
- uint32_t degree, Weighting weighting) :
- mDegree(degree), mWeighting(weighting) {
- clear();
-}
+LeastSquaresVelocityTrackerStrategy::LeastSquaresVelocityTrackerStrategy(uint32_t degree,
+ Weighting weighting)
+ : mDegree(degree), mWeighting(weighting), mIndex(0) {}
LeastSquaresVelocityTrackerStrategy::~LeastSquaresVelocityTrackerStrategy() {
}
-void LeastSquaresVelocityTrackerStrategy::clear() {
- mIndex = 0;
- mMovements[0].idBits.clear();
-}
-
void LeastSquaresVelocityTrackerStrategy::clearPointers(BitSet32 idBits) {
BitSet32 remainingIdBits(mMovements[mIndex].idBits.value & ~idBits.value);
mMovements[mIndex].idBits = remainingIdBits;
@@ -825,10 +811,6 @@
IntegratingVelocityTrackerStrategy::~IntegratingVelocityTrackerStrategy() {
}
-void IntegratingVelocityTrackerStrategy::clear() {
- mPointerIdBits.clear();
-}
-
void IntegratingVelocityTrackerStrategy::clearPointers(BitSet32 idBits) {
mPointerIdBits.value &= ~idBits.value;
}
@@ -920,18 +902,11 @@
// --- LegacyVelocityTrackerStrategy ---
-LegacyVelocityTrackerStrategy::LegacyVelocityTrackerStrategy() {
- clear();
-}
+LegacyVelocityTrackerStrategy::LegacyVelocityTrackerStrategy() : mIndex(0) {}
LegacyVelocityTrackerStrategy::~LegacyVelocityTrackerStrategy() {
}
-void LegacyVelocityTrackerStrategy::clear() {
- mIndex = 0;
- mMovements[0].idBits.clear();
-}
-
void LegacyVelocityTrackerStrategy::clearPointers(BitSet32 idBits) {
BitSet32 remainingIdBits(mMovements[mIndex].idBits.value & ~idBits.value);
mMovements[mIndex].idBits = remainingIdBits;
@@ -1029,18 +1004,11 @@
// --- ImpulseVelocityTrackerStrategy ---
-ImpulseVelocityTrackerStrategy::ImpulseVelocityTrackerStrategy() {
- clear();
-}
+ImpulseVelocityTrackerStrategy::ImpulseVelocityTrackerStrategy() : mIndex(0) {}
ImpulseVelocityTrackerStrategy::~ImpulseVelocityTrackerStrategy() {
}
-void ImpulseVelocityTrackerStrategy::clear() {
- mIndex = 0;
- mMovements[0].idBits.clear();
-}
-
void ImpulseVelocityTrackerStrategy::clearPointers(BitSet32 idBits) {
BitSet32 remainingIdBits(mMovements[mIndex].idBits.value & ~idBits.value);
mMovements[mIndex].idBits = remainingIdBits;
diff --git a/libs/input/tests/VelocityTracker_test.cpp b/libs/input/tests/VelocityTracker_test.cpp
index f8e5052..0b7def3 100644
--- a/libs/input/tests/VelocityTracker_test.cpp
+++ b/libs/input/tests/VelocityTracker_test.cpp
@@ -308,6 +308,27 @@
EXPECT_FALSE(computedVelocity.getVelocity(AMOTION_EVENT_AXIS_SCROLL, DEFAULT_POINTER_ID));
}
+TEST_F(VelocityTrackerTest, TestApiInteractionsWithNoMotionEvents) {
+ VelocityTracker vt(VelocityTracker::Strategy::DEFAULT);
+
+ EXPECT_FALSE(vt.getVelocity(AMOTION_EVENT_AXIS_X, DEFAULT_POINTER_ID));
+
+ VelocityTracker::Estimator estimator;
+ EXPECT_FALSE(vt.getEstimator(AMOTION_EVENT_AXIS_X, DEFAULT_POINTER_ID, &estimator));
+
+ VelocityTracker::ComputedVelocity computedVelocity = vt.getComputedVelocity(1000, 1000);
+ for (uint32_t id = 0; id <= MAX_POINTER_ID; id++) {
+ EXPECT_FALSE(computedVelocity.getVelocity(AMOTION_EVENT_AXIS_X, id));
+ EXPECT_FALSE(computedVelocity.getVelocity(AMOTION_EVENT_AXIS_Y, id));
+ }
+
+ EXPECT_EQ(-1, vt.getActivePointerId());
+
+ // Make sure that the clearing functions execute without an issue.
+ vt.clearPointers(BitSet32(7U));
+ vt.clear();
+}
+
TEST_F(VelocityTrackerTest, ThreePointsPositiveVelocityTest) {
// Same coordinate is reported 2 times in a row
// It is difficult to determine the correct answer here, but at least the direction