Skip to content

Commit 3a57dcd

Browse files
Treehugger RobotAndroid (Google) Code Review
Treehugger Robot
authored and
Android (Google) Code Review
committed
Merge changes I7f8683f7,I7417c007 into main
* changes: Refactor processing of relative pointer movements Refactor PointerChoreographer lock
2 parents 2721b5c + 5cd7497 commit 3a57dcd

2 files changed

Lines changed: 121 additions & 107 deletions

File tree

‎services/inputflinger/PointerChoreographer.cpp‎

Lines changed: 67 additions & 67 deletions
Original file line numberDiff line numberDiff line change
@@ -139,23 +139,24 @@ PointerChoreographer::PointerChoreographer(
139139
mShowTouchesEnabled(false),
140140
mStylusPointerIconEnabled(false),
141141
mCurrentFocusedDisplay(ui::LogicalDisplayId::DEFAULT),
142+
mIsWindowInfoListenerRegistered(false),
143+
mWindowInfoListener(sp<PointerChoreographerDisplayInfoListener>::make(this)),
142144
mRegisterListener(registerListener),
143145
mUnregisterListener(unregisterListener) {}
144146

145147
PointerChoreographer::~PointerChoreographer() {
146-
std::scoped_lock _l(mLock);
147-
if (mWindowInfoListener == nullptr) {
148-
return;
148+
if (mIsWindowInfoListenerRegistered) {
149+
mUnregisterListener(mWindowInfoListener);
150+
mIsWindowInfoListenerRegistered = false;
149151
}
150152
mWindowInfoListener->onPointerChoreographerDestroyed();
151-
mUnregisterListener(mWindowInfoListener);
152153
}
153154

154155
void PointerChoreographer::notifyInputDevicesChanged(const NotifyInputDevicesChangedArgs& args) {
155156
PointerDisplayChange pointerDisplayChange;
156157

157158
{ // acquire lock
158-
std::scoped_lock _l(mLock);
159+
std::scoped_lock _l(getLock());
159160

160161
mInputDeviceInfos = args.inputDeviceInfos;
161162
pointerDisplayChange = updatePointerControllersLocked();
@@ -191,7 +192,7 @@ void PointerChoreographer::fadeMouseCursorOnKeyPress(const android::NotifyKeyArg
191192
return;
192193
}
193194

194-
std::scoped_lock _l(mLock);
195+
std::scoped_lock _l(getLock());
195196
ui::LogicalDisplayId targetDisplay = args.displayId;
196197
if (targetDisplay == ui::LogicalDisplayId::INVALID) {
197198
targetDisplay = mCurrentFocusedDisplay;
@@ -204,7 +205,7 @@ void PointerChoreographer::fadeMouseCursorOnKeyPress(const android::NotifyKeyArg
204205
}
205206

206207
NotifyMotionArgs PointerChoreographer::processMotion(const NotifyMotionArgs& args) {
207-
std::scoped_lock _l(mLock);
208+
std::scoped_lock _l(getLock());
208209

209210
if (isFromMouse(args)) {
210211
return processMouseEventLocked(args);
@@ -242,14 +243,7 @@ NotifyMotionArgs PointerChoreographer::processMouseEventLocked(const NotifyMotio
242243
pc.setPosition(args.xCursorPosition, args.yCursorPosition);
243244
} else {
244245
// This is a relative mouse, so move the cursor by the specified amount.
245-
const float deltaX = args.pointerCoords[0].getAxisValue(AMOTION_EVENT_AXIS_RELATIVE_X);
246-
const float deltaY = args.pointerCoords[0].getAxisValue(AMOTION_EVENT_AXIS_RELATIVE_Y);
247-
pc.move(deltaX, deltaY);
248-
const auto [x, y] = pc.getPosition();
249-
newArgs.pointerCoords[0].setAxisValue(AMOTION_EVENT_AXIS_X, x);
250-
newArgs.pointerCoords[0].setAxisValue(AMOTION_EVENT_AXIS_Y, y);
251-
newArgs.xCursorPosition = x;
252-
newArgs.yCursorPosition = y;
246+
processPointerDeviceMotionEventLocked(/*byref*/ newArgs, /*byref*/ pc);
253247
}
254248
if (canUnfadeOnDisplay(displayId)) {
255249
pc.unfade(PointerControllerInterface::Transition::IMMEDIATE);
@@ -265,24 +259,9 @@ NotifyMotionArgs PointerChoreographer::processTouchpadEventLocked(const NotifyMo
265259
newArgs.displayId = displayId;
266260
if (args.getPointerCount() == 1 && args.classification == MotionClassification::NONE) {
267261
// This is a movement of the mouse pointer.
268-
const float deltaX = args.pointerCoords[0].getAxisValue(AMOTION_EVENT_AXIS_RELATIVE_X);
269-
const float deltaY = args.pointerCoords[0].getAxisValue(AMOTION_EVENT_AXIS_RELATIVE_Y);
270-
pc.move(deltaX, deltaY);
271-
if (canUnfadeOnDisplay(displayId)) {
272-
pc.unfade(PointerControllerInterface::Transition::IMMEDIATE);
273-
}
274-
275-
const auto [x, y] = pc.getPosition();
276-
newArgs.pointerCoords[0].setAxisValue(AMOTION_EVENT_AXIS_X, x);
277-
newArgs.pointerCoords[0].setAxisValue(AMOTION_EVENT_AXIS_Y, y);
278-
newArgs.xCursorPosition = x;
279-
newArgs.yCursorPosition = y;
262+
processPointerDeviceMotionEventLocked(/*byref*/ newArgs, /*byref*/ pc);
280263
} else {
281264
// This is a trackpad gesture with fake finger(s) that should not move the mouse pointer.
282-
if (canUnfadeOnDisplay(displayId)) {
283-
pc.unfade(PointerControllerInterface::Transition::IMMEDIATE);
284-
}
285-
286265
const auto [x, y] = pc.getPosition();
287266
for (uint32_t i = 0; i < newArgs.getPointerCount(); i++) {
288267
newArgs.pointerCoords[i].setAxisValue(AMOTION_EVENT_AXIS_X,
@@ -293,9 +272,25 @@ NotifyMotionArgs PointerChoreographer::processTouchpadEventLocked(const NotifyMo
293272
newArgs.xCursorPosition = x;
294273
newArgs.yCursorPosition = y;
295274
}
275+
if (canUnfadeOnDisplay(displayId)) {
276+
pc.unfade(PointerControllerInterface::Transition::IMMEDIATE);
277+
}
296278
return newArgs;
297279
}
298280

281+
void PointerChoreographer::processPointerDeviceMotionEventLocked(NotifyMotionArgs& newArgs,
282+
PointerControllerInterface& pc) {
283+
const float deltaX = newArgs.pointerCoords[0].getAxisValue(AMOTION_EVENT_AXIS_RELATIVE_X);
284+
const float deltaY = newArgs.pointerCoords[0].getAxisValue(AMOTION_EVENT_AXIS_RELATIVE_Y);
285+
286+
pc.move(deltaX, deltaY);
287+
const auto [x, y] = pc.getPosition();
288+
newArgs.pointerCoords[0].setAxisValue(AMOTION_EVENT_AXIS_X, x);
289+
newArgs.pointerCoords[0].setAxisValue(AMOTION_EVENT_AXIS_Y, y);
290+
newArgs.xCursorPosition = x;
291+
newArgs.yCursorPosition = y;
292+
}
293+
299294
void PointerChoreographer::processDrawingTabletEventLocked(const android::NotifyMotionArgs& args) {
300295
if (args.displayId == ui::LogicalDisplayId::INVALID) {
301296
return;
@@ -433,7 +428,7 @@ void PointerChoreographer::notifyDeviceReset(const NotifyDeviceResetArgs& args)
433428
}
434429

435430
void PointerChoreographer::processDeviceReset(const NotifyDeviceResetArgs& args) {
436-
std::scoped_lock _l(mLock);
431+
std::scoped_lock _l(getLock());
437432
mTouchPointersByDevice.erase(args.deviceId);
438433
mStylusPointersByDevice.erase(args.deviceId);
439434
mDrawingTabletPointersByDevice.erase(args.deviceId);
@@ -447,17 +442,22 @@ void PointerChoreographer::onControllerAddedOrRemovedLocked() {
447442
bool requireListener = !mTouchPointersByDevice.empty() || !mMousePointersByDisplay.empty() ||
448443
!mDrawingTabletPointersByDevice.empty() || !mStylusPointersByDevice.empty();
449444

450-
if (requireListener && mWindowInfoListener == nullptr) {
451-
mWindowInfoListener = sp<PointerChoreographerDisplayInfoListener>::make(this);
452-
mWindowInfoListener->setInitialDisplayInfos(mRegisterListener(mWindowInfoListener));
453-
onPrivacySensitiveDisplaysChangedLocked(mWindowInfoListener->getPrivacySensitiveDisplays());
454-
} else if (!requireListener && mWindowInfoListener != nullptr) {
445+
// PointerChoreographer uses Listener's lock which is already held by caller
446+
base::ScopedLockAssertion assumeLocked(mWindowInfoListener->mLock);
447+
448+
if (requireListener && !mIsWindowInfoListenerRegistered) {
449+
mIsWindowInfoListenerRegistered = true;
450+
mWindowInfoListener->setInitialDisplayInfosLocked(mRegisterListener(mWindowInfoListener));
451+
onPrivacySensitiveDisplaysChangedLocked(
452+
mWindowInfoListener->getPrivacySensitiveDisplaysLocked());
453+
} else if (!requireListener && mIsWindowInfoListenerRegistered) {
454+
mIsWindowInfoListenerRegistered = false;
455455
mUnregisterListener(mWindowInfoListener);
456-
mWindowInfoListener = nullptr;
457-
} else if (requireListener && mWindowInfoListener != nullptr) {
456+
} else if (requireListener) {
458457
// controller may have been added to an existing privacy sensitive display, we need to
459458
// update all controllers again
460-
onPrivacySensitiveDisplaysChangedLocked(mWindowInfoListener->getPrivacySensitiveDisplays());
459+
onPrivacySensitiveDisplaysChangedLocked(
460+
mWindowInfoListener->getPrivacySensitiveDisplaysLocked());
461461
}
462462
}
463463

@@ -494,22 +494,16 @@ void PointerChoreographer::onPrivacySensitiveDisplaysChangedLocked(
494494
void PointerChoreographer::notifyPointerCaptureChanged(
495495
const NotifyPointerCaptureChangedArgs& args) {
496496
if (args.request.isEnable()) {
497-
std::scoped_lock _l(mLock);
497+
std::scoped_lock _l(getLock());
498498
for (const auto& [_, mousePointerController] : mMousePointersByDisplay) {
499499
mousePointerController->fade(PointerControllerInterface::Transition::IMMEDIATE);
500500
}
501501
}
502502
mNextListener.notify(args);
503503
}
504504

505-
void PointerChoreographer::onPrivacySensitiveDisplaysChanged(
506-
const std::unordered_set<ui::LogicalDisplayId>& privacySensitiveDisplays) {
507-
std::scoped_lock _l(mLock);
508-
onPrivacySensitiveDisplaysChangedLocked(privacySensitiveDisplays);
509-
}
510-
511505
void PointerChoreographer::dump(std::string& dump) {
512-
std::scoped_lock _l(mLock);
506+
std::scoped_lock _l(getLock());
513507

514508
dump += "PointerChoreographer:\n";
515509
dump += StringPrintf(INDENT "Show Touches Enabled: %s\n",
@@ -579,6 +573,10 @@ bool PointerChoreographer::canUnfadeOnDisplay(ui::LogicalDisplayId displayId) {
579573
return mDisplaysWithPointersHidden.find(displayId) == mDisplaysWithPointersHidden.end();
580574
}
581575

576+
std::mutex& PointerChoreographer::getLock() const {
577+
return mWindowInfoListener->mLock;
578+
}
579+
582580
PointerChoreographer::PointerDisplayChange PointerChoreographer::updatePointerControllersLocked() {
583581
std::set<ui::LogicalDisplayId /*displayId*/> mouseDisplaysToKeep;
584582
std::set<DeviceId> touchDevicesToKeep;
@@ -641,7 +639,7 @@ PointerChoreographer::PointerDisplayChange PointerChoreographer::updatePointerCo
641639
std::erase_if(mDrawingTabletPointersByDevice, [&drawingTabletDevicesToKeep](const auto& pair) {
642640
return drawingTabletDevicesToKeep.find(pair.first) == drawingTabletDevicesToKeep.end();
643641
});
644-
std::erase_if(mMouseDevices, [&](DeviceId id) REQUIRES(mLock) {
642+
std::erase_if(mMouseDevices, [&](DeviceId id) REQUIRES(getLock()) {
645643
return std::find_if(mInputDeviceInfos.begin(), mInputDeviceInfos.end(),
646644
[id](const auto& info) { return info.getId() == id; }) ==
647645
mInputDeviceInfos.end();
@@ -677,7 +675,7 @@ void PointerChoreographer::setDefaultMouseDisplayId(ui::LogicalDisplayId display
677675
PointerDisplayChange pointerDisplayChange;
678676

679677
{ // acquire lock
680-
std::scoped_lock _l(mLock);
678+
std::scoped_lock _l(getLock());
681679

682680
mDefaultMouseDisplayId = displayId;
683681
pointerDisplayChange = updatePointerControllersLocked();
@@ -690,7 +688,7 @@ void PointerChoreographer::setDisplayViewports(const std::vector<DisplayViewport
690688
PointerDisplayChange pointerDisplayChange;
691689

692690
{ // acquire lock
693-
std::scoped_lock _l(mLock);
691+
std::scoped_lock _l(getLock());
694692
for (const auto& viewport : viewports) {
695693
const ui::LogicalDisplayId displayId = viewport.displayId;
696694
if (const auto it = mMousePointersByDisplay.find(displayId);
@@ -719,7 +717,7 @@ void PointerChoreographer::setDisplayViewports(const std::vector<DisplayViewport
719717

720718
std::optional<DisplayViewport> PointerChoreographer::getViewportForPointerDevice(
721719
ui::LogicalDisplayId associatedDisplayId) {
722-
std::scoped_lock _l(mLock);
720+
std::scoped_lock _l(getLock());
723721
const ui::LogicalDisplayId resolvedDisplayId = getTargetMouseDisplayLocked(associatedDisplayId);
724722
if (const auto viewport = findViewportByIdLocked(resolvedDisplayId); viewport) {
725723
return *viewport;
@@ -728,7 +726,7 @@ std::optional<DisplayViewport> PointerChoreographer::getViewportForPointerDevice
728726
}
729727

730728
FloatPoint PointerChoreographer::getMouseCursorPosition(ui::LogicalDisplayId displayId) {
731-
std::scoped_lock _l(mLock);
729+
std::scoped_lock _l(getLock());
732730
const ui::LogicalDisplayId resolvedDisplayId = getTargetMouseDisplayLocked(displayId);
733731
if (auto it = mMousePointersByDisplay.find(resolvedDisplayId);
734732
it != mMousePointersByDisplay.end()) {
@@ -741,7 +739,7 @@ void PointerChoreographer::setShowTouchesEnabled(bool enabled) {
741739
PointerDisplayChange pointerDisplayChange;
742740

743741
{ // acquire lock
744-
std::scoped_lock _l(mLock);
742+
std::scoped_lock _l(getLock());
745743
if (mShowTouchesEnabled == enabled) {
746744
return;
747745
}
@@ -756,7 +754,7 @@ void PointerChoreographer::setStylusPointerIconEnabled(bool enabled) {
756754
PointerDisplayChange pointerDisplayChange;
757755

758756
{ // acquire lock
759-
std::scoped_lock _l(mLock);
757+
std::scoped_lock _l(getLock());
760758
if (mStylusPointerIconEnabled == enabled) {
761759
return;
762760
}
@@ -770,7 +768,7 @@ void PointerChoreographer::setStylusPointerIconEnabled(bool enabled) {
770768
bool PointerChoreographer::setPointerIcon(
771769
std::variant<std::unique_ptr<SpriteIcon>, PointerIconStyle> icon,
772770
ui::LogicalDisplayId displayId, DeviceId deviceId) {
773-
std::scoped_lock _l(mLock);
771+
std::scoped_lock _l(getLock());
774772
if (deviceId < 0) {
775773
LOG(WARNING) << "Invalid device id " << deviceId << ". Cannot set pointer icon.";
776774
return false;
@@ -821,7 +819,7 @@ bool PointerChoreographer::setPointerIcon(
821819
}
822820

823821
void PointerChoreographer::setPointerIconVisibility(ui::LogicalDisplayId displayId, bool visible) {
824-
std::scoped_lock lock(mLock);
822+
std::scoped_lock lock(getLock());
825823
if (visible) {
826824
mDisplaysWithPointersHidden.erase(displayId);
827825
// We do not unfade the icons here, because we don't know when the last event happened.
@@ -843,14 +841,14 @@ void PointerChoreographer::setPointerIconVisibility(ui::LogicalDisplayId display
843841
}
844842

845843
void PointerChoreographer::setFocusedDisplay(ui::LogicalDisplayId displayId) {
846-
std::scoped_lock lock(mLock);
844+
std::scoped_lock lock(getLock());
847845
mCurrentFocusedDisplay = displayId;
848846
}
849847

850848
PointerChoreographer::ControllerConstructor PointerChoreographer::getMouseControllerConstructor(
851849
ui::LogicalDisplayId displayId) {
852850
std::function<std::shared_ptr<PointerControllerInterface>()> ctor =
853-
[this, displayId]() REQUIRES(mLock) {
851+
[this, displayId]() REQUIRES(getLock()) {
854852
auto pc = mPolicy.createPointerController(
855853
PointerControllerInterface::ControllerType::MOUSE);
856854
if (const auto viewport = findViewportByIdLocked(displayId); viewport) {
@@ -864,7 +862,7 @@ PointerChoreographer::ControllerConstructor PointerChoreographer::getMouseContro
864862
PointerChoreographer::ControllerConstructor PointerChoreographer::getStylusControllerConstructor(
865863
ui::LogicalDisplayId displayId) {
866864
std::function<std::shared_ptr<PointerControllerInterface>()> ctor =
867-
[this, displayId]() REQUIRES(mLock) {
865+
[this, displayId]() REQUIRES(getLock()) {
868866
auto pc = mPolicy.createPointerController(
869867
PointerControllerInterface::ControllerType::STYLUS);
870868
if (const auto viewport = findViewportByIdLocked(displayId); viewport) {
@@ -875,35 +873,37 @@ PointerChoreographer::ControllerConstructor PointerChoreographer::getStylusContr
875873
return ConstructorDelegate(std::move(ctor));
876874
}
877875

876+
// --- PointerChoreographer::PointerChoreographerDisplayInfoListener ---
877+
878878
void PointerChoreographer::PointerChoreographerDisplayInfoListener::onWindowInfosChanged(
879879
const gui::WindowInfosUpdate& windowInfosUpdate) {
880-
std::scoped_lock _l(mListenerLock);
880+
std::scoped_lock _l(mLock);
881881
if (mPointerChoreographer == nullptr) {
882882
return;
883883
}
884884
auto newPrivacySensitiveDisplays =
885885
getPrivacySensitiveDisplaysFromWindowInfos(windowInfosUpdate.windowInfos);
886886
if (newPrivacySensitiveDisplays != mPrivacySensitiveDisplays) {
887887
mPrivacySensitiveDisplays = std::move(newPrivacySensitiveDisplays);
888-
mPointerChoreographer->onPrivacySensitiveDisplaysChanged(mPrivacySensitiveDisplays);
888+
// PointerChoreographer uses Listener's lock.
889+
base::ScopedLockAssertion assumeLocked(mPointerChoreographer->getLock());
890+
mPointerChoreographer->onPrivacySensitiveDisplaysChangedLocked(mPrivacySensitiveDisplays);
889891
}
890892
}
891893

892-
void PointerChoreographer::PointerChoreographerDisplayInfoListener::setInitialDisplayInfos(
894+
void PointerChoreographer::PointerChoreographerDisplayInfoListener::setInitialDisplayInfosLocked(
893895
const std::vector<gui::WindowInfo>& windowInfos) {
894-
std::scoped_lock _l(mListenerLock);
895896
mPrivacySensitiveDisplays = getPrivacySensitiveDisplaysFromWindowInfos(windowInfos);
896897
}
897898

898899
std::unordered_set<ui::LogicalDisplayId /*displayId*/>
899-
PointerChoreographer::PointerChoreographerDisplayInfoListener::getPrivacySensitiveDisplays() {
900-
std::scoped_lock _l(mListenerLock);
900+
PointerChoreographer::PointerChoreographerDisplayInfoListener::getPrivacySensitiveDisplaysLocked() {
901901
return mPrivacySensitiveDisplays;
902902
}
903903

904904
void PointerChoreographer::PointerChoreographerDisplayInfoListener::
905905
onPointerChoreographerDestroyed() {
906-
std::scoped_lock _l(mListenerLock);
906+
std::scoped_lock _l(mLock);
907907
mPointerChoreographer = nullptr;
908908
}
909909

0 commit comments

Comments
 (0)