Merge "Improve VelocityTracker Strategy Handling"

3 files changed, 66 insertions, 78 deletions

diff --git a/include/input/VelocityTracker.h b/include/input/VelocityTracker.h
index 6144f5be4d..4251f0417d 100644
--- a/include/input/VelocityTracker.h
+++ b/include/input/VelocityTracker.h
@@ -145,16 +145,12 @@ public:
     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;
@@ -162,9 +158,17 @@ private:
     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);
 };
@@ -180,7 +184,6 @@ protected:
 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;
@@ -212,7 +215,6 @@ public:
     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;
@@ -253,7 +255,6 @@ public:
     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;
@@ -286,7 +287,6 @@ public:
     LegacyVelocityTrackerStrategy();
     virtual ~LegacyVelocityTrackerStrategy();
 
-    virtual void clear();
     virtual void clearPointers(BitSet32 idBits);
     void addMovement(nsecs_t eventTime, BitSet32 idBits,
                      const std::vector<float>& positions) override;
@@ -319,7 +319,6 @@ public:
     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 6f88a3860b..527a75b2cf 100644
--- a/libs/input/VelocityTracker.cpp
+++ b/libs/input/VelocityTracker.cpp
@@ -125,39 +125,32 @@ static std::string matrixToString(const float* a, uint32_t m, uint32_t n, bool r
 
 // --- 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 @@ std::unique_ptr<VelocityTrackerStrategy> VelocityTracker::createStrategy(
 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 @@ void VelocityTracker::clearPointers(BitSet32 idBits) {
         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 @@ void VelocityTracker::addMovement(nsecs_t eventTime, BitSet32 idBits,
 
         // 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 @@ void VelocityTracker::addMovement(nsecs_t eventTime, BitSet32 idBits,
         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 @@ void VelocityTracker::addMovement(const MotionEvent* event) {
             // 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 @@ std::optional<float> VelocityTracker::getVelocity(int32_t axis, uint32_t id) con
 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 @@ VelocityTracker::ComputedVelocity VelocityTracker::getComputedVelocity(int32_t u
 }
 
 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(uint32_t
 IntegratingVelocityTrackerStrategy::~IntegratingVelocityTrackerStrategy() {
 }
 
-void IntegratingVelocityTrackerStrategy::clear() {
-    mPointerIdBits.clear();
-}
-
 void IntegratingVelocityTrackerStrategy::clearPointers(BitSet32 idBits) {
     mPointerIdBits.value &= ~idBits.value;
 }
@@ -920,18 +902,11 @@ void IntegratingVelocityTrackerStrategy::populateEstimator(const State& state,
 
 // --- 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 @@ bool LegacyVelocityTrackerStrategy::getEstimator(uint32_t id,
 
 // --- 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 f8e505299c..0b7def372b 100644
--- a/libs/input/tests/VelocityTracker_test.cpp
+++ b/libs/input/tests/VelocityTracker_test.cpp
@@ -308,6 +308,27 @@ TEST_F(VelocityTrackerTest, TestGetComputedVelocity) {
     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