Address additional comments: Pointer icon refactor for touch/stylus
Bug: 293587049
Test: atest inputflinger_tests
Change-Id: Ic628105276fb06770932f05907598e95fcc1cafb
diff --git a/services/inputflinger/PointerChoreographer.cpp b/services/inputflinger/PointerChoreographer.cpp
index 841d95b..e529bdd 100644
--- a/services/inputflinger/PointerChoreographer.cpp
+++ b/services/inputflinger/PointerChoreographer.cpp
@@ -45,7 +45,11 @@
PointerChoreographer::PointerChoreographer(InputListenerInterface& listener,
PointerChoreographerPolicyInterface& policy)
- : mNextListener(listener),
+ : mTouchControllerConstructor([this]() REQUIRES(mLock) {
+ return mPolicy.createPointerController(
+ PointerControllerInterface::ControllerType::TOUCH);
+ }),
+ mNextListener(listener),
mPolicy(policy),
mDefaultMouseDisplayId(ADISPLAY_ID_DEFAULT),
mNotifiedPointerDisplayId(ADISPLAY_ID_NONE),
@@ -141,8 +145,7 @@
}
// Get the touch pointer controller for the device, or create one if it doesn't exist.
- auto [it, _] =
- mTouchPointersByDevice.try_emplace(args.deviceId, getTouchControllerConstructor());
+ auto [it, _] = mTouchPointersByDevice.try_emplace(args.deviceId, mTouchControllerConstructor);
PointerControllerInterface& pc = *it->second;
@@ -213,28 +216,8 @@
void PointerChoreographer::processDeviceReset(const NotifyDeviceResetArgs& args) {
std::scoped_lock _l(mLock);
-
- const InputDeviceInfo* info = findInputDeviceLocked(args.deviceId);
- if (info == nullptr) {
- return;
- }
-
- const uint32_t sources = info->getSources();
- const int32_t displayId = info->getAssociatedDisplayId();
- if (isFromSource(sources, AINPUT_SOURCE_TOUCHSCREEN) && mShowTouchesEnabled &&
- displayId != ADISPLAY_ID_NONE) {
- if (const auto it = mTouchPointersByDevice.find(args.deviceId);
- it != mTouchPointersByDevice.end()) {
- it->second->clearSpots();
- }
- }
- if (isFromSource(info->getSources(), AINPUT_SOURCE_STYLUS) && mStylusPointerIconEnabled &&
- displayId != ADISPLAY_ID_NONE) {
- if (const auto it = mStylusPointersByDevice.find(args.deviceId);
- it != mStylusPointersByDevice.end()) {
- it->second->fade(PointerControllerInterface::Transition::IMMEDIATE);
- }
- }
+ mTouchPointersByDevice.erase(args.deviceId);
+ mStylusPointersByDevice.erase(args.deviceId);
}
void PointerChoreographer::notifyPointerCaptureChanged(
@@ -288,12 +271,9 @@
}
InputDeviceInfo* PointerChoreographer::findInputDeviceLocked(DeviceId deviceId) {
- for (auto& info : mInputDeviceInfos) {
- if (info.getId() == deviceId) {
- return &info;
- }
- }
- return nullptr;
+ auto it = std::find_if(mInputDeviceInfos.begin(), mInputDeviceInfos.end(),
+ [deviceId](const auto& info) { return info.getId() == deviceId; });
+ return it != mInputDeviceInfos.end() ? &(*it) : nullptr;
}
void PointerChoreographer::updatePointerControllersLocked() {
@@ -321,30 +301,14 @@
}
// Remove PointerControllers no longer needed.
- // This has the side-effect of fading pointers or clearing spots before removal.
std::erase_if(mMousePointersByDisplay, [&mouseDisplaysToKeep](const auto& pair) {
- auto& [displayId, controller] = pair;
- if (mouseDisplaysToKeep.find(displayId) == mouseDisplaysToKeep.end()) {
- controller->fade(PointerControllerInterface::Transition::IMMEDIATE);
- return true;
- }
- return false;
+ return mouseDisplaysToKeep.find(pair.first) == mouseDisplaysToKeep.end();
});
std::erase_if(mTouchPointersByDevice, [&touchDevicesToKeep](const auto& pair) {
- auto& [deviceId, controller] = pair;
- if (touchDevicesToKeep.find(deviceId) == touchDevicesToKeep.end()) {
- controller->clearSpots();
- return true;
- }
- return false;
+ return touchDevicesToKeep.find(pair.first) == touchDevicesToKeep.end();
});
std::erase_if(mStylusPointersByDevice, [&stylusDevicesToKeep](const auto& pair) {
- auto& [deviceId, controller] = pair;
- if (stylusDevicesToKeep.find(deviceId) == stylusDevicesToKeep.end()) {
- controller->fade(PointerControllerInterface::Transition::IMMEDIATE);
- return true;
- }
- return false;
+ return stylusDevicesToKeep.find(pair.first) == stylusDevicesToKeep.end();
});
// Notify the policy if there's a change on the pointer display ID.
@@ -449,13 +413,6 @@
return ConstructorDelegate(std::move(ctor));
}
-PointerChoreographer::ControllerConstructor PointerChoreographer::getTouchControllerConstructor() {
- std::function<std::shared_ptr<PointerControllerInterface>()> ctor = [this]() REQUIRES(mLock) {
- return mPolicy.createPointerController(PointerControllerInterface::ControllerType::TOUCH);
- };
- return ConstructorDelegate(std::move(ctor));
-}
-
PointerChoreographer::ControllerConstructor PointerChoreographer::getStylusControllerConstructor(
int32_t displayId) {
std::function<std::shared_ptr<PointerControllerInterface>()> ctor =
diff --git a/services/inputflinger/PointerChoreographer.h b/services/inputflinger/PointerChoreographer.h
index 90569d8..26d2fef 100644
--- a/services/inputflinger/PointerChoreographer.h
+++ b/services/inputflinger/PointerChoreographer.h
@@ -105,8 +105,8 @@
using ControllerConstructor =
ConstructorDelegate<std::function<std::shared_ptr<PointerControllerInterface>()>>;
+ ControllerConstructor mTouchControllerConstructor GUARDED_BY(mLock);
ControllerConstructor getMouseControllerConstructor(int32_t displayId) REQUIRES(mLock);
- ControllerConstructor getTouchControllerConstructor() REQUIRES(mLock);
ControllerConstructor getStylusControllerConstructor(int32_t displayId) REQUIRES(mLock);
std::mutex mLock;
diff --git a/services/inputflinger/tests/FakePointerController.cpp b/services/inputflinger/tests/FakePointerController.cpp
index 57d00a0..5475594 100644
--- a/services/inputflinger/tests/FakePointerController.cpp
+++ b/services/inputflinger/tests/FakePointerController.cpp
@@ -57,6 +57,12 @@
ASSERT_NEAR(y, actualY, 1);
}
+void FakePointerController::assertSpotCount(int32_t displayId, int32_t count) {
+ auto it = mSpotsByDisplay.find(displayId);
+ ASSERT_TRUE(it != mSpotsByDisplay.end()) << "Spots not found for display " << displayId;
+ ASSERT_EQ(static_cast<size_t>(count), it->second.size());
+}
+
bool FakePointerController::isPointerShown() {
return mIsPointerShown;
}
diff --git a/services/inputflinger/tests/FakePointerController.h b/services/inputflinger/tests/FakePointerController.h
index c75f6ed..d7e40b3 100644
--- a/services/inputflinger/tests/FakePointerController.h
+++ b/services/inputflinger/tests/FakePointerController.h
@@ -37,6 +37,7 @@
void setDisplayViewport(const DisplayViewport& viewport) override;
void assertPosition(float x, float y);
+ void assertSpotCount(int32_t displayId, int32_t count);
bool isPointerShown();
private:
diff --git a/services/inputflinger/tests/PointerChoreographer_test.cpp b/services/inputflinger/tests/PointerChoreographer_test.cpp
index 1a52729..68f5857 100644
--- a/services/inputflinger/tests/PointerChoreographer_test.cpp
+++ b/services/inputflinger/tests/PointerChoreographer_test.cpp
@@ -589,7 +589,6 @@
mChoreographer.notifyInputDevicesChanged(
{/*id=*/0, {generateTestDeviceInfo(DEVICE_ID, AINPUT_SOURCE_TOUCHSCREEN, DISPLAY_ID)}});
mChoreographer.setShowTouchesEnabled(true);
- assertPointerControllerNotCreated();
// Emit touch event. Now PointerController should be created.
mChoreographer.notifyMotion(
@@ -624,7 +623,6 @@
mChoreographer.notifyInputDevicesChanged(
{/*id=*/0, {generateTestDeviceInfo(DEVICE_ID, AINPUT_SOURCE_TOUCHSCREEN, DISPLAY_ID)}});
mChoreographer.setShowTouchesEnabled(true);
- assertPointerControllerNotCreated();
mChoreographer.notifyMotion(
MotionArgsBuilder(AMOTION_EVENT_ACTION_DOWN, AINPUT_SOURCE_TOUCHSCREEN)
.pointer(FIRST_TOUCH_POINTER)
@@ -670,9 +668,7 @@
.displayId(DISPLAY_ID)
.build());
auto pc = assertPointerControllerCreated(ControllerType::TOUCH);
- auto it = pc->getSpots().find(DISPLAY_ID);
- ASSERT_TRUE(it != pc->getSpots().end());
- ASSERT_EQ(size_t(1), it->second.size());
+ pc->assertSpotCount(DISPLAY_ID, 1);
// Emit second pointer down.
mChoreographer.notifyMotion(
@@ -684,9 +680,7 @@
.deviceId(DEVICE_ID)
.displayId(DISPLAY_ID)
.build());
- it = pc->getSpots().find(DISPLAY_ID);
- ASSERT_TRUE(it != pc->getSpots().end());
- ASSERT_EQ(size_t(2), it->second.size());
+ pc->assertSpotCount(DISPLAY_ID, 2);
// Emit second pointer up.
mChoreographer.notifyMotion(
@@ -698,9 +692,7 @@
.deviceId(DEVICE_ID)
.displayId(DISPLAY_ID)
.build());
- it = pc->getSpots().find(DISPLAY_ID);
- ASSERT_TRUE(it != pc->getSpots().end());
- ASSERT_EQ(size_t(1), it->second.size());
+ pc->assertSpotCount(DISPLAY_ID, 1);
// Emit first pointer up.
mChoreographer.notifyMotion(
@@ -709,9 +701,7 @@
.deviceId(DEVICE_ID)
.displayId(DISPLAY_ID)
.build());
- it = pc->getSpots().find(DISPLAY_ID);
- ASSERT_TRUE(it != pc->getSpots().end());
- ASSERT_EQ(size_t(0), it->second.size());
+ pc->assertSpotCount(DISPLAY_ID, 0);
}
TEST_F(PointerChoreographerTest, TouchSetsSpotsForStylusEvent) {
@@ -729,9 +719,7 @@
.displayId(DISPLAY_ID)
.build());
auto pc = assertPointerControllerCreated(ControllerType::TOUCH);
- auto it = pc->getSpots().find(DISPLAY_ID);
- ASSERT_TRUE(it != pc->getSpots().end());
- ASSERT_EQ(size_t(1), it->second.size());
+ pc->assertSpotCount(DISPLAY_ID, 1);
}
TEST_F(PointerChoreographerTest, TouchSetsSpotsForTwoDisplays) {
@@ -751,9 +739,7 @@
.displayId(DISPLAY_ID)
.build());
auto firstDisplayPc = assertPointerControllerCreated(ControllerType::TOUCH);
- auto firstSpotsIt = firstDisplayPc->getSpots().find(DISPLAY_ID);
- ASSERT_TRUE(firstSpotsIt != firstDisplayPc->getSpots().end());
- ASSERT_EQ(size_t(1), firstSpotsIt->second.size());
+ firstDisplayPc->assertSpotCount(DISPLAY_ID, 1);
// Emit touch events with second device.
mChoreographer.notifyMotion(
@@ -774,14 +760,10 @@
auto secondDisplayPc = assertPointerControllerCreated(ControllerType::TOUCH);
// Check if the spots are set for the second device.
- auto secondSpotsIt = secondDisplayPc->getSpots().find(ANOTHER_DISPLAY_ID);
- ASSERT_TRUE(secondSpotsIt != secondDisplayPc->getSpots().end());
- ASSERT_EQ(size_t(2), secondSpotsIt->second.size());
+ secondDisplayPc->assertSpotCount(ANOTHER_DISPLAY_ID, 2);
// Check if there's no change on the spot of the first device.
- firstSpotsIt = firstDisplayPc->getSpots().find(DISPLAY_ID);
- ASSERT_TRUE(firstSpotsIt != firstDisplayPc->getSpots().end());
- ASSERT_EQ(size_t(1), firstSpotsIt->second.size());
+ firstDisplayPc->assertSpotCount(DISPLAY_ID, 1);
}
TEST_F(PointerChoreographerTest, WhenTouchDeviceIsResetClearsSpots) {
@@ -796,14 +778,11 @@
.displayId(DISPLAY_ID)
.build());
auto pc = assertPointerControllerCreated(ControllerType::TOUCH);
- auto it = pc->getSpots().find(DISPLAY_ID);
- ASSERT_TRUE(it != pc->getSpots().end());
- ASSERT_EQ(size_t(1), it->second.size());
+ pc->assertSpotCount(DISPLAY_ID, 1);
- // Reset the device and see there's no spot.
+ // Reset the device and ensure the touch pointer controller was removed.
mChoreographer.notifyDeviceReset(NotifyDeviceResetArgs(/*id=*/1, /*eventTime=*/0, DEVICE_ID));
- it = pc->getSpots().find(DISPLAY_ID);
- ASSERT_TRUE(it == pc->getSpots().end());
+ assertPointerControllerRemoved(pc);
}
TEST_F(PointerChoreographerTest,
@@ -859,7 +838,6 @@
mChoreographer.notifyInputDevicesChanged(
{/*id=*/0, {generateTestDeviceInfo(DEVICE_ID, AINPUT_SOURCE_STYLUS, DISPLAY_ID)}});
mChoreographer.setStylusPointerIconEnabled(true);
- assertPointerControllerNotCreated();
mChoreographer.notifyMotion(
MotionArgsBuilder(AMOTION_EVENT_ACTION_HOVER_ENTER, AINPUT_SOURCE_STYLUS)
.pointer(STYLUS_POINTER)
@@ -878,7 +856,6 @@
mChoreographer.notifyInputDevicesChanged(
{/*id=*/0, {generateTestDeviceInfo(DEVICE_ID, AINPUT_SOURCE_STYLUS, DISPLAY_ID)}});
mChoreographer.setStylusPointerIconEnabled(true);
- assertPointerControllerNotCreated();
mChoreographer.notifyMotion(
MotionArgsBuilder(AMOTION_EVENT_ACTION_HOVER_ENTER, AINPUT_SOURCE_STYLUS)
.pointer(STYLUS_POINTER)
@@ -900,7 +877,6 @@
mChoreographer.notifyInputDevicesChanged(
{/*id=*/0, {generateTestDeviceInfo(DEVICE_ID, AINPUT_SOURCE_STYLUS, DISPLAY_ID)}});
mChoreographer.setStylusPointerIconEnabled(true);
- assertPointerControllerNotCreated();
mChoreographer.notifyMotion(
MotionArgsBuilder(AMOTION_EVENT_ACTION_HOVER_ENTER, AINPUT_SOURCE_STYLUS)
.pointer(STYLUS_POINTER)
@@ -918,7 +894,6 @@
mChoreographer.notifyInputDevicesChanged(
{/*id=*/0, {generateTestDeviceInfo(DEVICE_ID, AINPUT_SOURCE_STYLUS, DISPLAY_ID)}});
mChoreographer.setStylusPointerIconEnabled(true);
- assertPointerControllerNotCreated();
mChoreographer.notifyMotion(
MotionArgsBuilder(AMOTION_EVENT_ACTION_HOVER_ENTER, AINPUT_SOURCE_STYLUS)
.pointer(STYLUS_POINTER)
@@ -943,7 +918,6 @@
mChoreographer.notifyInputDevicesChanged(
{/*id=*/0, {generateTestDeviceInfo(DEVICE_ID, AINPUT_SOURCE_STYLUS, DISPLAY_ID)}});
mChoreographer.setStylusPointerIconEnabled(true);
- assertPointerControllerNotCreated();
mChoreographer.notifyMotion(
MotionArgsBuilder(AMOTION_EVENT_ACTION_HOVER_ENTER, AINPUT_SOURCE_STYLUS)
.pointer(STYLUS_POINTER)
@@ -1056,7 +1030,7 @@
ASSERT_TRUE(firstDisplayPc->isPointerShown());
}
-TEST_F(PointerChoreographerTest, WhenStylusDeviceIsResetFadesPointer) {
+TEST_F(PointerChoreographerTest, WhenStylusDeviceIsResetRemovesPointer) {
// Make sure the PointerController is created and there is a pointer.
mChoreographer.notifyInputDevicesChanged(
{/*id=*/0, {generateTestDeviceInfo(DEVICE_ID, AINPUT_SOURCE_STYLUS, DISPLAY_ID)}});
@@ -1071,9 +1045,9 @@
auto pc = assertPointerControllerCreated(ControllerType::STYLUS);
ASSERT_TRUE(pc->isPointerShown());
- // Reset the device and see the pointer disappeared.
+ // Reset the device and see the pointer controller was removed.
mChoreographer.notifyDeviceReset(NotifyDeviceResetArgs(/*id=*/1, /*eventTime=*/0, DEVICE_ID));
- ASSERT_FALSE(pc->isPointerShown());
+ assertPointerControllerRemoved(pc);
}
} // namespace android