Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit d4fd3a11 authored by Prabir Pradhan's avatar Prabir Pradhan
Browse files

Synchronize pointer display change requests

Previously, when InputManagerService requests for PointerController to
change the pointer display, there was no way to know when the request
was completed or whether it succeeded. This could lead to a few issues:

- WM's MousePositionTracker's coordinates would not be updated until the
  next mouse event was generated, meaning the position would be out of
  sync.
- The creation of a virtual mouse device moves the pointer to a specific
  displayId. In order to test this behavior, we would need to sleep in
  the test code to wait for the system to update the pointer display and
  position, resulting in generally flaky tests.

Here, we add a way to synchonize changes to the pointer display so that
InputMangerService can know the current pointer display with certainty.
PointerController, which is updated in the InputReader thread, is the
source of truth of the pointer display. We add a policy call to notify
IMS when the pointer display changes.

When the pointer display is changed, the cursor position on the updated
display is also updated so that the VirtualMouse#getCursorPosition() API
is synchronized to the pointer display change.

Bug: 216792538
Test: atest FrameworksServicesTests:InputManagerServiceTests
Test: atest PointerIconTest
Change-Id: I578fd1aba9335e2e078d749321e55a6d05299f3b
Merged-In: I578fd1aba9335e2e078d749321e55a6d05299f3b
parent eb2d1e65
Loading
Loading
Loading
Loading
+8 −1
Original line number Diff line number Diff line
@@ -75,8 +75,15 @@ public abstract class InputManagerInternal {
    /**
     * Sets the display id that the MouseCursorController will be forced to target. Pass
     * {@link android.view.Display#INVALID_DISPLAY} to clear the override.
     *
     * Note: This method generally blocks until the pointer display override has propagated.
     * When setting a new override, the caller should ensure that an input device that can control
     * the mouse pointer is connected. If a new override is set when no such input device is
     * connected, the caller may be blocked for an arbitrary period of time.
     *
     * @return true if the pointer displayId was set successfully, or false if it fails.
     */
    public abstract void setVirtualMousePointerDisplayId(int pointerDisplayId);
    public abstract boolean setVirtualMousePointerDisplayId(int pointerDisplayId);

    /**
     * Gets the display id that the MouseCursorController is being forced to target. Returns
+7 −0
Original line number Diff line number Diff line
@@ -106,6 +106,7 @@ PointerController::PointerController(const sp<PointerControllerPolicyInterface>&
PointerController::~PointerController() {
    mDisplayInfoListener->onPointerControllerDestroyed();
    mUnregisterWindowInfosListener(mDisplayInfoListener);
    mContext.getPolicy()->onPointerDisplayIdChanged(ADISPLAY_ID_NONE, 0, 0);
}

std::mutex& PointerController::getLock() const {
@@ -255,6 +256,12 @@ void PointerController::setDisplayViewport(const DisplayViewport& viewport) {
        getAdditionalMouseResources = true;
    }
    mCursorController.setDisplayViewport(viewport, getAdditionalMouseResources);
    if (viewport.displayId != mLocked.pointerDisplayId) {
        float xPos, yPos;
        mCursorController.getPosition(&xPos, &yPos);
        mContext.getPolicy()->onPointerDisplayIdChanged(viewport.displayId, xPos, yPos);
        mLocked.pointerDisplayId = viewport.displayId;
    }
}

void PointerController::updatePointerIcon(int32_t iconId) {
+1 −0
Original line number Diff line number Diff line
@@ -104,6 +104,7 @@ private:

    struct Locked {
        Presentation presentation;
        int32_t pointerDisplayId = ADISPLAY_ID_NONE;

        std::vector<gui::DisplayInfo> mDisplayInfos;
        std::unordered_map<int32_t /* displayId */, TouchSpotController> spotControllers;
+1 −0
Original line number Diff line number Diff line
@@ -79,6 +79,7 @@ public:
            std::map<int32_t, PointerAnimation>* outAnimationResources, int32_t displayId) = 0;
    virtual int32_t getDefaultPointerIconId() = 0;
    virtual int32_t getCustomPointerIconId() = 0;
    virtual void onPointerDisplayIdChanged(int32_t displayId, float xPos, float yPos) = 0;
};

/*
+37 −3
Original line number Diff line number Diff line
@@ -56,9 +56,11 @@ public:
            std::map<int32_t, PointerAnimation>* outAnimationResources, int32_t displayId) override;
    virtual int32_t getDefaultPointerIconId() override;
    virtual int32_t getCustomPointerIconId() override;
    virtual void onPointerDisplayIdChanged(int32_t displayId, float xPos, float yPos) override;

    bool allResourcesAreLoaded();
    bool noResourcesAreLoaded();
    std::optional<int32_t> getLastReportedPointerDisplayId() { return latestPointerDisplayId; }

private:
    void loadPointerIconForType(SpriteIcon* icon, int32_t cursorType);
@@ -66,6 +68,7 @@ private:
    bool pointerIconLoaded{false};
    bool pointerResourcesLoaded{false};
    bool additionalMouseResourcesLoaded{false};
    std::optional<int32_t /*displayId*/> latestPointerDisplayId;
};

void MockPointerControllerPolicyInterface::loadPointerIcon(SpriteIcon* icon, int32_t) {
@@ -126,12 +129,19 @@ void MockPointerControllerPolicyInterface::loadPointerIconForType(SpriteIcon* ic
    icon->hotSpotX = hotSpot.first;
    icon->hotSpotY = hotSpot.second;
}

void MockPointerControllerPolicyInterface::onPointerDisplayIdChanged(int32_t displayId,
                                                                     float /*xPos*/,
                                                                     float /*yPos*/) {
    latestPointerDisplayId = displayId;
}

class PointerControllerTest : public Test {
protected:
    PointerControllerTest();
    ~PointerControllerTest();

    void ensureDisplayViewportIsSet();
    void ensureDisplayViewportIsSet(int32_t displayId = ADISPLAY_ID_DEFAULT);

    sp<MockSprite> mPointerSprite;
    sp<MockPointerControllerPolicyInterface> mPolicy;
@@ -168,9 +178,9 @@ PointerControllerTest::~PointerControllerTest() {
    mThread.join();
}

void PointerControllerTest::ensureDisplayViewportIsSet() {
void PointerControllerTest::ensureDisplayViewportIsSet(int32_t displayId) {
    DisplayViewport viewport;
    viewport.displayId = ADISPLAY_ID_DEFAULT;
    viewport.displayId = displayId;
    viewport.logicalRight = 1600;
    viewport.logicalBottom = 1200;
    viewport.physicalRight = 800;
@@ -255,6 +265,30 @@ TEST_F(PointerControllerTest, doesNotGetResourcesBeforeSettingViewport) {
    ensureDisplayViewportIsSet();
}

TEST_F(PointerControllerTest, notifiesPolicyWhenPointerDisplayChanges) {
    EXPECT_FALSE(mPolicy->getLastReportedPointerDisplayId())
            << "A pointer display change does not occur when PointerController is created.";

    ensureDisplayViewportIsSet(ADISPLAY_ID_DEFAULT);

    const auto lastReportedPointerDisplayId = mPolicy->getLastReportedPointerDisplayId();
    ASSERT_TRUE(lastReportedPointerDisplayId)
            << "The policy is notified of a pointer display change when the viewport is first set.";
    EXPECT_EQ(ADISPLAY_ID_DEFAULT, *lastReportedPointerDisplayId)
            << "Incorrect pointer display notified.";

    ensureDisplayViewportIsSet(42);

    EXPECT_EQ(42, *mPolicy->getLastReportedPointerDisplayId())
            << "The policy is notified when the pointer display changes.";

    // Release the PointerController.
    mPointerController = nullptr;

    EXPECT_EQ(ADISPLAY_ID_NONE, *mPolicy->getLastReportedPointerDisplayId())
            << "The pointer display changes to invalid when PointerController is destroyed.";
}

class PointerControllerWindowInfoListenerTest : public Test {};

class TestPointerController : public PointerController {
Loading