From c32a411f0d7c5811c5c95429f251a7d9c9c84d5c Mon Sep 17 00:00:00 2001 From: Prabir Pradhan Date: Tue, 23 Jan 2024 23:20:37 +0000 Subject: InputDispatcher: Improve pointer transform mapping in InputTarget Bug: 316355518 Bug: 210460522 Test: atest inputflinger_tests Change-Id: I14f77d6138ad2458649650fd09cef9253a669363 --- .../inputflinger/dispatcher/InputDispatcher.cpp | 52 ++++++++---------- services/inputflinger/dispatcher/InputTarget.cpp | 62 ++++++++++++++++------ services/inputflinger/dispatcher/InputTarget.h | 27 +++++++--- 3 files changed, 86 insertions(+), 55 deletions(-) diff --git a/services/inputflinger/dispatcher/InputDispatcher.cpp b/services/inputflinger/dispatcher/InputDispatcher.cpp index 67190060e7..ce9c6e027e 100644 --- a/services/inputflinger/dispatcher/InputDispatcher.cpp +++ b/services/inputflinger/dispatcher/InputDispatcher.cpp @@ -393,22 +393,23 @@ std::unique_ptr createDispatchEntry(const InputTarget& inputTarge // as long as all other pointers are normalized to the same value and the final DispatchEntry // uses the transform for the normalized pointer. const ui::Transform& firstPointerTransform = - inputTarget.pointerTransforms[firstMarkedBit(inputTarget.pointerIds)]; - ui::Transform inverseFirstTransform = firstPointerTransform.inverse(); + inputTarget.getTransformForPointer(firstMarkedBit(inputTarget.getPointerIds())); + const ui::Transform inverseFirstTransform = firstPointerTransform.inverse(); // Iterate through all pointers in the event to normalize against the first. - for (uint32_t pointerIndex = 0; pointerIndex < motionEntry.getPointerCount(); pointerIndex++) { - const PointerProperties& pointerProperties = motionEntry.pointerProperties[pointerIndex]; - uint32_t pointerId = uint32_t(pointerProperties.id); - const ui::Transform& currTransform = inputTarget.pointerTransforms[pointerId]; + for (size_t i = 0; i < motionEntry.getPointerCount(); i++) { + PointerCoords& newCoords = pointerCoords[i]; + + const auto pointerId = motionEntry.pointerProperties[i].id; + const ui::Transform& currTransform = inputTarget.getTransformForPointer(pointerId); - pointerCoords[pointerIndex].copyFrom(motionEntry.pointerCoords[pointerIndex]); + newCoords.copyFrom(motionEntry.pointerCoords[i]); // First, apply the current pointer's transform to update the coordinates into // window space. - pointerCoords[pointerIndex].transform(currTransform); + newCoords.transform(currTransform); // Next, apply the inverse transform of the normalized coordinates so the // current coordinates are transformed into the normalized coordinate space. - pointerCoords[pointerIndex].transform(inverseFirstTransform); + newCoords.transform(inverseFirstTransform); } std::unique_ptr combinedMotionEntry = @@ -1630,14 +1631,12 @@ void InputDispatcher::dispatchFocusLocked(nsecs_t currentTime, if (connection == nullptr) { return; // Connection has gone away } - InputTarget target; - target.connection = connection; entry->dispatchInProgress = true; std::string message = std::string("Focus ") + (entry->hasFocus ? "entering " : "leaving ") + connection->getInputChannelName(); std::string reason = std::string("reason=").append(entry->reason); android_log_event_list(LOGTAG_INPUT_FOCUS) << message << reason << LOG_ID_EVENTS; - dispatchEventLocked(currentTime, entry, {target}); + dispatchEventLocked(currentTime, entry, {{connection}}); } void InputDispatcher::dispatchPointerCaptureChangedLocked( @@ -1703,10 +1702,8 @@ void InputDispatcher::dispatchPointerCaptureChangedLocked( } return; } - InputTarget target; - target.connection = connection; entry->dispatchInProgress = true; - dispatchEventLocked(currentTime, entry, {target}); + dispatchEventLocked(currentTime, entry, {{connection}}); dropReason = DropReason::NOT_DROPPED; } @@ -1739,9 +1736,7 @@ std::vector InputDispatcher::getInputTargetsFromWindowHandlesLocked if (connection == nullptr) { continue; // Connection has gone away } - InputTarget target; - target.connection = connection; - inputTargets.push_back(target); + inputTargets.emplace_back(connection); } return inputTargets; } @@ -2022,10 +2017,8 @@ void InputDispatcher::dispatchDragLocked(nsecs_t currentTime, if (connection == nullptr) { return; // Connection has gone away } - InputTarget target; - target.connection = connection; entry->dispatchInProgress = true; - dispatchEventLocked(currentTime, entry, {target}); + dispatchEventLocked(currentTime, entry, {{connection}}); } void InputDispatcher::logOutboundMotionDetails(const char* prefix, const MotionEntry& entry) { @@ -2868,8 +2861,7 @@ std::optional InputDispatcher::createInputTargetLocked( ALOGW("Not creating InputTarget for %s, no input channel", windowHandle->getName().c_str()); return {}; } - InputTarget inputTarget; - inputTarget.connection = connection; + InputTarget inputTarget{connection}; inputTarget.windowHandle = windowHandle; inputTarget.dispatchMode = dispatchMode; inputTarget.flags = targetFlags; @@ -2982,8 +2974,7 @@ void InputDispatcher::addGlobalMonitoringTargetsLocked(std::vector& if (monitorsIt == mGlobalMonitorsByDisplay.end()) return; for (const Monitor& monitor : selectResponsiveMonitorsLocked(monitorsIt->second)) { - InputTarget target; - target.connection = monitor.connection; + InputTarget target{monitor.connection}; // target.firstDownTimeInTarget is not set for global monitors. It is only required in split // touch and global monitoring works as intended even without setting firstDownTimeInTarget if (const auto& it = mDisplayInfos.find(displayId); it != mDisplayInfos.end()) { @@ -3275,7 +3266,7 @@ void InputDispatcher::prepareDispatchCycleLocked(nsecs_t currentTime, ALOGD("channel '%s' ~ prepareDispatchCycle - flags=%s, " "globalScaleFactor=%f, pointerIds=%s %s", connection->getInputChannelName().c_str(), inputTarget.flags.string().c_str(), - inputTarget.globalScaleFactor, bitsetToString(inputTarget.pointerIds).c_str(), + inputTarget.globalScaleFactor, bitsetToString(inputTarget.getPointerIds()).c_str(), inputTarget.getPointerInfoString().c_str()); } @@ -3297,7 +3288,7 @@ void InputDispatcher::prepareDispatchCycleLocked(nsecs_t currentTime, ftl::enum_string(eventEntry->type).c_str()); const MotionEntry& originalMotionEntry = static_cast(*eventEntry); - if (inputTarget.pointerIds.count() != originalMotionEntry.getPointerCount()) { + if (inputTarget.getPointerIds().count() != originalMotionEntry.getPointerCount()) { if (!inputTarget.firstDownTimeInTarget.has_value()) { logDispatchStateLocked(); LOG(FATAL) << "Splitting motion events requires a down time to be set for the " @@ -3306,7 +3297,7 @@ void InputDispatcher::prepareDispatchCycleLocked(nsecs_t currentTime, << originalMotionEntry.getDescription(); } std::unique_ptr splitMotionEntry = - splitMotionEvent(originalMotionEntry, inputTarget.pointerIds, + splitMotionEvent(originalMotionEntry, inputTarget.getPointerIds(), inputTarget.firstDownTimeInTarget.value()); if (!splitMotionEntry) { return; // split event was dropped @@ -4107,7 +4098,7 @@ void InputDispatcher::synthesizeCancelationEventsForConnectionLocked( const bool wasEmpty = connection->outboundQueue.empty(); // The target to use if we don't find a window associated with the channel. - const InputTarget fallbackTarget{.connection = connection}; + const InputTarget fallbackTarget{connection}; const auto& token = connection->getToken(); for (size_t i = 0; i < cancelationEvents.size(); i++) { @@ -4225,8 +4216,7 @@ void InputDispatcher::synthesizePointerDownEventsForConnectionLocked( targetFlags, pointerIds, motionEntry.downTime, targets); } else { - targets.emplace_back( - InputTarget{.connection = connection, .flags = targetFlags}); + targets.emplace_back(connection, targetFlags); const auto it = mDisplayInfos.find(motionEntry.displayId); if (it != mDisplayInfos.end()) { targets.back().displayTransform = it->second.transform; diff --git a/services/inputflinger/dispatcher/InputTarget.cpp b/services/inputflinger/dispatcher/InputTarget.cpp index 28e3c6d69a..35ad858736 100644 --- a/services/inputflinger/dispatcher/InputTarget.cpp +++ b/services/inputflinger/dispatcher/InputTarget.cpp @@ -26,6 +26,15 @@ using android::base::StringPrintf; namespace android::inputdispatcher { +namespace { + +const static ui::Transform kIdentityTransform{}; + +} + +InputTarget::InputTarget(const std::shared_ptr& connection, ftl::Flags flags) + : connection(connection), flags(flags) {} + void InputTarget::addPointers(std::bitset newPointerIds, const ui::Transform& transform) { // The pointerIds can be empty, but still a valid InputTarget. This can happen when there is no @@ -36,31 +45,46 @@ void InputTarget::addPointers(std::bitset newPointerIds, } // Ensure that the new set of pointers doesn't overlap with the current set of pointers. - if ((pointerIds & newPointerIds).any()) { + if ((getPointerIds() & newPointerIds).any()) { LOG(FATAL) << __func__ << " - overlap with incoming pointers " << bitsetToString(newPointerIds) << " in " << *this; } - pointerIds |= newPointerIds; - for (size_t i = 0; i < newPointerIds.size(); i++) { - if (!newPointerIds.test(i)) { - continue; + for (auto& [existingTransform, existingPointers] : mPointerTransforms) { + if (transform == existingTransform) { + existingPointers |= newPointerIds; + return; } - pointerTransforms[i] = transform; } + mPointerTransforms.emplace_back(transform, newPointerIds); } void InputTarget::setDefaultPointerTransform(const ui::Transform& transform) { - pointerIds.reset(); - pointerTransforms[0] = transform; + mPointerTransforms = {{transform, {}}}; } bool InputTarget::useDefaultPointerTransform() const { - return pointerIds.none(); + return mPointerTransforms.size() <= 1; } const ui::Transform& InputTarget::getDefaultPointerTransform() const { - return pointerTransforms[0]; + if (!useDefaultPointerTransform()) { + LOG(FATAL) << __func__ << ": Not using default pointer transform"; + } + return mPointerTransforms.size() == 1 ? mPointerTransforms[0].first : kIdentityTransform; +} + +const ui::Transform& InputTarget::getTransformForPointer(int32_t pointerId) const { + for (const auto& [transform, ids] : mPointerTransforms) { + if (ids.test(pointerId)) { + return transform; + } + } + + LOG(FATAL) << __func__ + << ": Cannot get transform: The following Pointer ID does not exist in target: " + << pointerId; + return kIdentityTransform; } std::string InputTarget::getPointerInfoString() const { @@ -71,17 +95,21 @@ std::string InputTarget::getPointerInfoString() const { return out; } - for (uint32_t i = 0; i < pointerIds.size(); i++) { - if (!pointerIds.test(i)) { - continue; - } - - const std::string name = "pointerId " + std::to_string(i) + ":"; - pointerTransforms[i].dump(out, name.c_str(), " "); + for (const auto& [transform, ids] : mPointerTransforms) { + const std::string name = "pointerIds " + bitsetToString(ids) + ":"; + transform.dump(out, name.c_str(), " "); } return out; } +std::bitset InputTarget::getPointerIds() const { + PointerIds allIds; + for (const auto& [_, ids] : mPointerTransforms) { + allIds |= ids; + } + return allIds; +} + std::ostream& operator<<(std::ostream& out, const InputTarget& target) { out << "{connection="; if (target.connection != nullptr) { diff --git a/services/inputflinger/dispatcher/InputTarget.h b/services/inputflinger/dispatcher/InputTarget.h index 5728bdf1e8..058639a742 100644 --- a/services/inputflinger/dispatcher/InputTarget.h +++ b/services/inputflinger/dispatcher/InputTarget.h @@ -33,7 +33,8 @@ namespace android::inputdispatcher { * be added to input event coordinates to compensate for the absolute position of the * window area. */ -struct InputTarget { +class InputTarget { +public: using Flags = InputTargetFlags; enum class DispatchMode { @@ -80,20 +81,17 @@ struct InputTarget { // Current display transform. Used for compatibility for raw coordinates. ui::Transform displayTransform; - // The subset of pointer ids to include in motion events dispatched to this input target - // if FLAG_SPLIT is set. - std::bitset pointerIds; // Event time for the first motion event (ACTION_DOWN) dispatched to this input target if // FLAG_SPLIT is set. std::optional firstDownTimeInTarget; - // The data is stored by the pointerId. Use the bit position of pointerIds to look up - // Transform per pointerId. - ui::Transform pointerTransforms[MAX_POINTERS]; // The window that this input target is being dispatched to. It is possible for this to be // null for cases like global monitors. sp windowHandle; + InputTarget() = default; + InputTarget(const std::shared_ptr&, ftl::Flags = {}); + void addPointers(std::bitset pointerIds, const ui::Transform& transform); void setDefaultPointerTransform(const ui::Transform& transform); @@ -111,7 +109,22 @@ struct InputTarget { */ const ui::Transform& getDefaultPointerTransform() const; + const ui::Transform& getTransformForPointer(int32_t pointerId) const; + + std::bitset getPointerIds() const; + std::string getPointerInfoString() const; + +private: + template + using ArrayMap = std::vector>; + using PointerIds = std::bitset; + // The mapping of pointer IDs to the transform that should be used for that collection of IDs. + // Each of the pointer IDs are mutually disjoint, and their union makes up pointer IDs to + // include in the motion events dispatched to this target. We use an ArrayMap to store this to + // avoid having to define hash or comparison functions for ui::Transform, which would be needed + // to use std::unordered_map or std::map respectively. + ArrayMap mPointerTransforms; }; std::ostream& operator<<(std::ostream& out, const InputTarget& target); -- cgit v1.2.3-59-g8ed1b From c88b1fa7cb48ce2a28d1e6c4db4b9c002f4f407a Mon Sep 17 00:00:00 2001 From: Prabir Pradhan Date: Tue, 23 Jan 2024 21:48:03 +0000 Subject: InputDispatcher: Create new EventEntries for events with zero-ed coords The InputConfig flag ZERO_COORDS modifieds the coordinates of an event before sending them to windows. Treat these events as new synthesized events by creating a new EventEntry when zeroing the coords. Bug: 316355518 Bug: 210460522 Test: atest inputflinger_tests Change-Id: I8c50a5977f7a491a7b73a8bacf2a0087135108fb --- .../inputflinger/dispatcher/InputDispatcher.cpp | 73 +++++++++++----------- 1 file changed, 38 insertions(+), 35 deletions(-) diff --git a/services/inputflinger/dispatcher/InputDispatcher.cpp b/services/inputflinger/dispatcher/InputDispatcher.cpp index ce9c6e027e..0ab6af757d 100644 --- a/services/inputflinger/dispatcher/InputDispatcher.cpp +++ b/services/inputflinger/dispatcher/InputDispatcher.cpp @@ -365,17 +365,20 @@ size_t firstMarkedBit(T set) { return i; } -std::unique_ptr createDispatchEntry(const InputTarget& inputTarget, +std::unique_ptr createDispatchEntry(const IdGenerator& idGenerator, + const InputTarget& inputTarget, std::shared_ptr eventEntry, ftl::Flags inputTargetFlags, int64_t vsyncId) { + const bool zeroCoords = inputTargetFlags.test(InputTarget::Flags::ZERO_COORDS); const sp win = inputTarget.windowHandle; const std::optional windowId = win ? std::make_optional(win->getInfo()->id) : std::nullopt; // Assume the only targets that are not associated with a window are global monitors, and use // the system UID for global monitors for tracing purposes. const gui::Uid uid = win ? win->getInfo()->ownerUid : gui::Uid(AID_SYSTEM); - if (inputTarget.useDefaultPointerTransform()) { + + if (inputTarget.useDefaultPointerTransform() && !zeroCoords) { const ui::Transform& transform = inputTarget.getDefaultPointerTransform(); return std::make_unique(eventEntry, inputTargetFlags, transform, inputTarget.displayTransform, @@ -386,34 +389,39 @@ std::unique_ptr createDispatchEntry(const InputTarget& inputTarge ALOG_ASSERT(eventEntry->type == EventEntry::Type::MOTION); const MotionEntry& motionEntry = static_cast(*eventEntry); - std::vector pointerCoords; - pointerCoords.resize(motionEntry.getPointerCount()); - - // Use the first pointer information to normalize all other pointers. This could be any pointer - // as long as all other pointers are normalized to the same value and the final DispatchEntry - // uses the transform for the normalized pointer. - const ui::Transform& firstPointerTransform = - inputTarget.getTransformForPointer(firstMarkedBit(inputTarget.getPointerIds())); - const ui::Transform inverseFirstTransform = firstPointerTransform.inverse(); - - // Iterate through all pointers in the event to normalize against the first. - for (size_t i = 0; i < motionEntry.getPointerCount(); i++) { - PointerCoords& newCoords = pointerCoords[i]; - - const auto pointerId = motionEntry.pointerProperties[i].id; - const ui::Transform& currTransform = inputTarget.getTransformForPointer(pointerId); + std::vector pointerCoords{motionEntry.getPointerCount()}; - newCoords.copyFrom(motionEntry.pointerCoords[i]); - // First, apply the current pointer's transform to update the coordinates into - // window space. - newCoords.transform(currTransform); - // Next, apply the inverse transform of the normalized coordinates so the - // current coordinates are transformed into the normalized coordinate space. - newCoords.transform(inverseFirstTransform); + const ui::Transform* transform = &kIdentityTransform; + const ui::Transform* displayTransform = &kIdentityTransform; + if (zeroCoords) { + std::for_each(pointerCoords.begin(), pointerCoords.end(), [](auto& pc) { pc.clear(); }); + } else { + // Use the first pointer information to normalize all other pointers. This could be any + // pointer as long as all other pointers are normalized to the same value and the final + // DispatchEntry uses the transform for the normalized pointer. + transform = + &inputTarget.getTransformForPointer(firstMarkedBit(inputTarget.getPointerIds())); + const ui::Transform inverseTransform = transform->inverse(); + displayTransform = &inputTarget.displayTransform; + + // Iterate through all pointers in the event to normalize against the first. + for (size_t i = 0; i < motionEntry.getPointerCount(); i++) { + PointerCoords& newCoords = pointerCoords[i]; + const auto pointerId = motionEntry.pointerProperties[i].id; + const ui::Transform& currTransform = inputTarget.getTransformForPointer(pointerId); + + newCoords.copyFrom(motionEntry.pointerCoords[i]); + // First, apply the current pointer's transform to update the coordinates into + // window space. + newCoords.transform(currTransform); + // Next, apply the inverse transform of the normalized coordinates so the + // current coordinates are transformed into the normalized coordinate space. + newCoords.transform(inverseTransform); + } } std::unique_ptr combinedMotionEntry = - std::make_unique(motionEntry.id, motionEntry.injectionState, + std::make_unique(idGenerator.nextId(), motionEntry.injectionState, motionEntry.eventTime, motionEntry.deviceId, motionEntry.source, motionEntry.displayId, motionEntry.policyFlags, motionEntry.action, @@ -427,7 +435,7 @@ std::unique_ptr createDispatchEntry(const InputTarget& inputTarge std::unique_ptr dispatchEntry = std::make_unique(std::move(combinedMotionEntry), inputTargetFlags, - firstPointerTransform, inputTarget.displayTransform, + *transform, *displayTransform, inputTarget.globalScaleFactor, uid, vsyncId, windowId); return dispatchEntry; } @@ -3355,7 +3363,8 @@ void InputDispatcher::enqueueDispatchEntryLocked(const std::shared_ptr dispatchEntry = - createDispatchEntry(inputTarget, eventEntry, inputTarget.flags, mWindowInfosVsyncId); + createDispatchEntry(mIdGenerator, inputTarget, eventEntry, inputTarget.flags, + mWindowInfosVsyncId); // Use the eventEntry from dispatchEntry since the entry may have changed and can now be a // different EventEntry than what was passed in. @@ -3473,7 +3482,7 @@ void InputDispatcher::enqueueDispatchEntryLocked(const std::shared_ptrgetInputChannelName() << " with event " << cancelEvent->getDescription(); std::unique_ptr cancelDispatchEntry = - createDispatchEntry(inputTarget, std::move(cancelEvent), + createDispatchEntry(mIdGenerator, inputTarget, std::move(cancelEvent), ftl::Flags(), mWindowInfosVsyncId); // Send these cancel events to the queue before sending the event from the new @@ -3653,12 +3662,6 @@ status_t InputDispatcher::publishMotionEvent(Connection& connection, } usingCoords = scaledCoords; } - } else if (dispatchEntry.targetFlags.test(InputTarget::Flags::ZERO_COORDS)) { - // We don't want the dispatch target to know the coordinates - for (uint32_t i = 0; i < motionEntry.getPointerCount(); i++) { - scaledCoords[i].clear(); - } - usingCoords = scaledCoords; } std::array hmac = getSignature(motionEntry, dispatchEntry); -- cgit v1.2.3-59-g8ed1b From 9d5f9ce1b7ca608784f1eb4e40ef12a576123456 Mon Sep 17 00:00:00 2001 From: Prabir Pradhan Date: Wed, 24 Jan 2024 00:03:41 +0000 Subject: InputDispatcher_test: Consume all moves to prevent events from batching InputDispatcherTest uses a real input channel, which means if subsequent MOVE events are not consumed, they will be batched together. Currently this applies to windows with split touches, where pointers moving on one split window will generate MOVEs on the other window which the test is not consuming, and thus end up getting batched. Avoid this situation by explicitly consuming all MOVE events in affected tests. Bug: 210460522 Test: atest inputflinger_tests Change-Id: I308c12e2aaf98f630bdac8021f7238e913a49056 --- .../inputflinger/tests/InputDispatcher_test.cpp | 44 +++++++++++++--------- 1 file changed, 27 insertions(+), 17 deletions(-) diff --git a/services/inputflinger/tests/InputDispatcher_test.cpp b/services/inputflinger/tests/InputDispatcher_test.cpp index 4c455f7804..eccbb57374 100644 --- a/services/inputflinger/tests/InputDispatcher_test.cpp +++ b/services/inputflinger/tests/InputDispatcher_test.cpp @@ -3865,48 +3865,57 @@ TEST_F(InputDispatcherTest, SplitTouchesSendCorrectActionDownTime) { // Touch down on the first window mDispatcher->notifyMotion(generateTouchArgs(AMOTION_EVENT_ACTION_DOWN, {{50, 50}})); - mDispatcher->waitForIdle(); - std::unique_ptr motionEvent1 = window1->consumeMotionEvent(); - ASSERT_NE(nullptr, motionEvent1); + const std::unique_ptr firstDown = + window1->consumeMotionEvent(AllOf(WithMotionAction(ACTION_DOWN))); + ASSERT_EQ(firstDown->getDownTime(), firstDown->getEventTime()); window2->assertNoEvents(); - nsecs_t downTimeForWindow1 = motionEvent1->getDownTime(); - ASSERT_EQ(motionEvent1->getDownTime(), motionEvent1->getEventTime()); // Now touch down on the window with another pointer mDispatcher->notifyMotion(generateTouchArgs(POINTER_1_DOWN, {{50, 50}, {150, 50}})); mDispatcher->waitForIdle(); - std::unique_ptr motionEvent2 = window2->consumeMotionEvent(); - ASSERT_NE(nullptr, motionEvent2); - nsecs_t downTimeForWindow2 = motionEvent2->getDownTime(); - ASSERT_NE(downTimeForWindow1, downTimeForWindow2); - ASSERT_EQ(motionEvent2->getDownTime(), motionEvent2->getEventTime()); + + const std::unique_ptr secondDown = + window2->consumeMotionEvent(AllOf(WithMotionAction(ACTION_DOWN))); + ASSERT_EQ(secondDown->getDownTime(), secondDown->getEventTime()); + ASSERT_NE(firstDown->getDownTime(), secondDown->getDownTime()); + // We currently send MOVE events to all windows receiving a split touch when there is any change + // in the touch state, even when none of the pointers in the split window actually moved. + // Document this behavior in the test. + window1->consumeMotionMove(); // Now move the pointer on the second window mDispatcher->notifyMotion(generateTouchArgs(AMOTION_EVENT_ACTION_MOVE, {{50, 50}, {151, 51}})); mDispatcher->waitForIdle(); - window2->consumeMotionEvent(WithDownTime(downTimeForWindow2)); + + window2->consumeMotionEvent(WithDownTime(secondDown->getDownTime())); + window1->consumeMotionEvent(WithDownTime(firstDown->getDownTime())); // Now add new touch down on the second window mDispatcher->notifyMotion(generateTouchArgs(POINTER_2_DOWN, {{50, 50}, {151, 51}, {150, 50}})); mDispatcher->waitForIdle(); - window2->consumeMotionEvent(WithDownTime(downTimeForWindow2)); - // TODO(b/232530217): do not send the unnecessary MOVE event and delete the next line - window1->consumeMotionMove(); - window1->assertNoEvents(); + window2->consumeMotionEvent( + AllOf(WithMotionAction(POINTER_1_DOWN), WithDownTime(secondDown->getDownTime()))); + window1->consumeMotionEvent(WithDownTime(firstDown->getDownTime())); // Now move the pointer on the first window mDispatcher->notifyMotion( generateTouchArgs(AMOTION_EVENT_ACTION_MOVE, {{51, 51}, {151, 51}, {150, 50}})); mDispatcher->waitForIdle(); - window1->consumeMotionEvent(WithDownTime(downTimeForWindow1)); + window1->consumeMotionEvent(WithDownTime(firstDown->getDownTime())); + window2->consumeMotionEvent(WithDownTime(secondDown->getDownTime())); + + // Now add new touch down on the first window mDispatcher->notifyMotion( generateTouchArgs(POINTER_3_DOWN, {{51, 51}, {151, 51}, {150, 50}, {50, 50}})); mDispatcher->waitForIdle(); - window1->consumeMotionEvent(WithDownTime(downTimeForWindow1)); + + window1->consumeMotionEvent( + AllOf(WithMotionAction(POINTER_1_DOWN), WithDownTime(firstDown->getDownTime()))); + window2->consumeMotionEvent(WithDownTime(secondDown->getDownTime())); } TEST_F(InputDispatcherTest, HoverMoveEnterMouseClickAndHoverMoveExit) { @@ -10780,6 +10789,7 @@ TEST_F(InputDispatcherDragTests, DragAndDropWhenSplitTouch) { InputEventInjectionSync::WAIT_FOR_RESULT)) << "Inject motion event should return InputEventInjectionResult::SUCCEEDED"; mWindow->consumeMotionDown(ADISPLAY_ID_DEFAULT); + mSecondWindow->consumeMotionMove(ADISPLAY_ID_DEFAULT); // Perform drag and drop from first window. ASSERT_TRUE(startDrag(false)); -- cgit v1.2.3-59-g8ed1b From 65a071a788a1f5e575d85460d08182d0726abc83 Mon Sep 17 00:00:00 2001 From: Prabir Pradhan Date: Fri, 5 Jan 2024 20:52:09 +0000 Subject: InputDispatcher_test: Verify all traced events match exactly Bug: 210460522 Test: atest inputflinger_tests Change-Id: I2ab660ed0a6888c23bc711fb8494385c22b3c404 --- include/input/Input.h | 10 +++- libs/input/Input.cpp | 27 ++++++++++ .../inputflinger/tests/FakeInputTracingBackend.cpp | 62 ++++++++++++++++++++-- .../inputflinger/tests/FakeInputTracingBackend.h | 4 +- 4 files changed, 95 insertions(+), 8 deletions(-) diff --git a/include/input/Input.h b/include/input/Input.h index 7b253a53ef..a84dcfc63c 100644 --- a/include/input/Input.h +++ b/include/input/Input.h @@ -180,7 +180,8 @@ static constexpr size_t MAX_POINTERS = 16; * Declare a concrete type for the NDK's input event forward declaration. */ struct AInputEvent { - virtual ~AInputEvent() { } + virtual ~AInputEvent() {} + bool operator==(const AInputEvent&) const = default; }; /* @@ -545,6 +546,8 @@ public: static int32_t nextId(); + bool operator==(const InputEvent&) const = default; + protected: void initialize(int32_t id, DeviceId deviceId, uint32_t source, int32_t displayId, std::array hmac); @@ -598,6 +601,8 @@ public: static const char* actionToString(int32_t action); + bool operator==(const KeyEvent&) const = default; + protected: int32_t mAction; int32_t mFlags; @@ -917,6 +922,9 @@ public: // The rounding precision for transformed motion events. static constexpr float ROUNDING_PRECISION = 0.001f; + bool operator==(const MotionEvent&) const; + inline bool operator!=(const MotionEvent& o) const { return !(*this == o); }; + protected: int32_t mAction; int32_t mActionButton; diff --git a/libs/input/Input.cpp b/libs/input/Input.cpp index 0d29b4db96..e831d2496f 100644 --- a/libs/input/Input.cpp +++ b/libs/input/Input.cpp @@ -1007,6 +1007,33 @@ PointerCoords MotionEvent::calculateTransformedCoords(uint32_t source, return out; } +bool MotionEvent::operator==(const android::MotionEvent& o) const { + // We use NaN values to represent invalid cursor positions. Since NaN values are not equal + // to themselves according to IEEE 754, we cannot use the default equality operator to compare + // MotionEvents. Therefore we define a custom equality operator with special handling for NaNs. + // clang-format off + return InputEvent::operator==(static_cast(o)) && + mAction == o.mAction && + mActionButton == o.mActionButton && + mFlags == o.mFlags && + mEdgeFlags == o.mEdgeFlags && + mMetaState == o.mMetaState && + mButtonState == o.mButtonState && + mClassification == o.mClassification && + mTransform == o.mTransform && + mXPrecision == o.mXPrecision && + mYPrecision == o.mYPrecision && + ((std::isnan(mRawXCursorPosition) && std::isnan(o.mRawXCursorPosition)) || + mRawXCursorPosition == o.mRawXCursorPosition) && + ((std::isnan(mRawYCursorPosition) && std::isnan(o.mRawYCursorPosition)) || + mRawYCursorPosition == o.mRawYCursorPosition) && + mRawTransform == o.mRawTransform && mDownTime == o.mDownTime && + mPointerProperties == o.mPointerProperties && + mSampleEventTimes == o.mSampleEventTimes && + mSamplePointerCoords == o.mSamplePointerCoords; + // clang-format on +} + std::ostream& operator<<(std::ostream& out, const MotionEvent& event) { out << "MotionEvent { action=" << MotionEvent::actionToString(event.getAction()); if (event.getActionButton() != 0) { diff --git a/services/inputflinger/tests/FakeInputTracingBackend.cpp b/services/inputflinger/tests/FakeInputTracingBackend.cpp index f4a06f7d28..b7af356eba 100644 --- a/services/inputflinger/tests/FakeInputTracingBackend.cpp +++ b/services/inputflinger/tests/FakeInputTracingBackend.cpp @@ -37,6 +37,30 @@ inline auto getId(const trace::TracedEvent& v) { return std::visit([](const auto& event) { return event.id; }, v); } +MotionEvent toInputEvent( + const trace::TracedMotionEvent& e, + const trace::InputTracingBackendInterface::WindowDispatchArgs& dispatchArgs, + const std::array& hmac) { + MotionEvent traced; + traced.initialize(e.id, e.deviceId, e.source, e.displayId, hmac, e.action, e.actionButton, + dispatchArgs.resolvedFlags, e.edgeFlags, e.metaState, e.buttonState, + e.classification, dispatchArgs.transform, e.xPrecision, e.yPrecision, + e.xCursorPosition, e.yCursorPosition, dispatchArgs.rawTransform, e.downTime, + e.eventTime, e.pointerProperties.size(), e.pointerProperties.data(), + e.pointerCoords.data()); + return traced; +} + +KeyEvent toInputEvent(const trace::TracedKeyEvent& e, + const trace::InputTracingBackendInterface::WindowDispatchArgs& dispatchArgs, + const std::array& hmac) { + KeyEvent traced; + traced.initialize(e.id, e.deviceId, e.source, e.displayId, hmac, e.action, + dispatchArgs.resolvedFlags, e.keyCode, e.scanCode, e.metaState, e.repeatCount, + e.downTime, e.eventTime); + return traced; +} + } // namespace // --- VerifyingTrace --- @@ -55,6 +79,7 @@ void VerifyingTrace::verifyExpectedEventsTraced() { std::unique_lock lock(mLock); base::ScopedLockAssertion assumeLocked(mLock); + // Poll for all expected events to be traced, and keep track of the latest poll result. base::Result result; mEventTracedCondition.wait_for(lock, TRACE_TIMEOUT, [&]() REQUIRES(mLock) { for (const auto& [expectedEvent, windowId] : mExpectedEvents) { @@ -101,12 +126,39 @@ base::Result VerifyingTrace::verifyEventTraced(const Event& expectedEvent, }); if (tracedDispatchesIt == mTracedWindowDispatches.end()) { msg << "Expected dispatch of event with ID 0x" << std::hex << expectedEvent.getId() - << " to window with ID 0x" << expectedWindowId << " to be traced, but it was not." - << "\nExpected event: " << expectedEvent; + << " to window with ID 0x" << expectedWindowId << " to be traced, but it was not.\n" + << "Expected event: " << expectedEvent; return error(msg); } - return {}; + // Verify that the traced event matches the expected event exactly. + return std::visit( + [&](const auto& traced) -> base::Result { + Event tracedEvent; + using T = std::decay_t; + if constexpr (std::is_same_v && + std::is_same_v) { + tracedEvent = + toInputEvent(traced, *tracedDispatchesIt, expectedEvent.getHmac()); + } else if constexpr (std::is_same_v && + std::is_same_v) { + tracedEvent = + toInputEvent(traced, *tracedDispatchesIt, expectedEvent.getHmac()); + } else { + msg << "Received the wrong event type!\n" + << "Expected event: " << expectedEvent; + return error(msg); + } + + const auto result = testing::internal::CmpHelperEQ("expectedEvent", "tracedEvent", + expectedEvent, tracedEvent); + if (!result) { + msg << result.failure_message(); + return error(msg); + } + return {}; + }, + tracedEventsIt->second); } // --- FakeInputTracingBackend --- @@ -114,7 +166,7 @@ base::Result VerifyingTrace::verifyEventTraced(const Event& expectedEvent, void FakeInputTracingBackend::traceKeyEvent(const trace::TracedKeyEvent& event) const { { std::scoped_lock lock(mTrace->mLock); - mTrace->mTracedEvents.emplace(event.id); + mTrace->mTracedEvents.emplace(event.id, event); } mTrace->mEventTracedCondition.notify_all(); } @@ -122,7 +174,7 @@ void FakeInputTracingBackend::traceKeyEvent(const trace::TracedKeyEvent& event) void FakeInputTracingBackend::traceMotionEvent(const trace::TracedMotionEvent& event) const { { std::scoped_lock lock(mTrace->mLock); - mTrace->mTracedEvents.emplace(event.id); + mTrace->mTracedEvents.emplace(event.id, event); } mTrace->mEventTracedCondition.notify_all(); } diff --git a/services/inputflinger/tests/FakeInputTracingBackend.h b/services/inputflinger/tests/FakeInputTracingBackend.h index 40ca3a29f8..108419aaaf 100644 --- a/services/inputflinger/tests/FakeInputTracingBackend.h +++ b/services/inputflinger/tests/FakeInputTracingBackend.h @@ -25,7 +25,7 @@ #include #include #include -#include +#include #include namespace android::inputdispatcher { @@ -58,7 +58,7 @@ public: private: std::mutex mLock; std::condition_variable mEventTracedCondition; - std::unordered_set mTracedEvents GUARDED_BY(mLock); + std::unordered_map mTracedEvents GUARDED_BY(mLock); using WindowDispatchArgs = trace::InputTracingBackendInterface::WindowDispatchArgs; std::vector mTracedWindowDispatches GUARDED_BY(mLock); std::vector, int32_t /*windowId*/>> -- cgit v1.2.3-59-g8ed1b