Separate QPD::tabletDevice into priv tabletDevice and queryTabletDevice

There doesn't seem to be any reason users will need to query tablet
devices by their IDs, because every event comes with a complete
instance already, and we have QInputDevice::devices() to list them all.

QPointingDevicePrivate::tabletDevice() can create a new instance if a
matching one is not found (and complains about that); it's intended
for use in QtGui, as a way to find the device if it was not part of the
QWSI event. Now it sets the parent of those auto-created instances
to QCoreApplication to avoid a memory leak.

On the other hand, queryTabletDevice() is intended for use in platform plugins
that need to check whether an instance exists; but they will take care
of creating new instances themselves, and thus have more control over the
parent and the details being stored. Now that the systemId can also be given,
the search is more likely to have a unique result, on window systems
that provide device IDs.

Rename id() to systemId() to clarify that it's a system-specific unique
device ID of some sort, not the same as the uniqueId that a stylus has.
However it seems that in practice, this will often be 0; so clarify that
if it's not unique, QInputDevicePrivate::fromId() and queryTabletDevice()
may not always find the right instance.

Clarify the function usage via comments.

Change-Id: I82bb8d1c26eeaf06f07c290828aa17ec4a31646b
Reviewed-by: Volker Hilsheimer <volker.hilsheimer@qt.io>

src/gui/kernel/qevent.cpp

@@ -2207,9 +2207,9 @@ QTabletEvent::QTabletEvent(Type type, const QPointF &pos, const QPointF &globalP
                            qreal rotation, int z, Qt::KeyboardModifiers keyState, qint64 uniqueID,
                            Qt::MouseButton button, Qt::MouseButtons buttons)
     : QTabletEvent(type,
-                    QPointingDevice::tabletDevice(QInputDevice::DeviceType(deviceType),
+                   QPointingDevicePrivate::tabletDevice(QInputDevice::DeviceType(deviceType),
-                                                  QPointingDevice::PointerType(pointerType),
+                                                        QPointingDevice::PointerType(pointerType),
-                                                  QPointingDeviceUniqueId::fromNumericId(uniqueID)),
+                                                        QPointingDeviceUniqueId::fromNumericId(uniqueID)),
                    pos, globalPos, pressure, xTilt, yTilt, tangentialPressure,
                    rotation, z, keyState, button, buttons)
 {

src/gui/kernel/qinputdevice.cpp

@@ -175,14 +175,14 @@ bool QInputDevice::hasCapability(QInputDevice::Capability capability) const
 }
 
 /*!
-    Returns the platform ID (for example xinput ID on the X11 platform).
+    Returns the platform specific system ID (for example xinput ID on the X11 platform).
 
-    All platforms are expected to provide a unique ID for each device.
+    All platforms are expected to provide a unique system ID for each device.
 */
-qint64 QInputDevice::id() const
+qint64 QInputDevice::systemId() const
 {
     Q_D(const QInputDevice);
-    return d->id;
+    return d->systemId;
 }
 
 /*!
@@ -265,17 +265,21 @@ bool QInputDevicePrivate::isRegistered(const QInputDevice *dev)
 
 /*!
     \internal
-    Find the device with the given \a id, which must be unique.
+    Find the device with the given \a systemId (for example the xinput
+    device ID on X11), which is expected to be unique if nonzero.
 
-    \note Use QPointingDevice::tabletDevice() if the device is a tablet
+    If the \a systemId is not unique, this function returns the first one found.
-    or a tablet stylus; in that case, \a id is not unique.
+
+    \note Use QInputDevicePrivate::queryTabletDevice() if the device is a
+    tablet or a tablet stylus; in that case, \a id is not unique.
 */
-const QInputDevice *QInputDevicePrivate::fromId(qint64 id)
+const QInputDevice *QInputDevicePrivate::fromId(qint64 systemId)
 {
     QMutexLocker locker(&devicesMutex);
-    for (const QInputDevice *dev : *deviceList())
+    for (const QInputDevice *dev : *deviceList()) {
-        if (const_cast<QInputDevicePrivate *>(QInputDevicePrivate::get(dev))->id == id)
+        if (dev->systemId() == systemId)
             return dev;
+    }
     return nullptr;
 }
 
@@ -296,7 +300,7 @@ void QInputDevicePrivate::unregisterDevice(const QInputDevice *dev)
 
 bool QInputDevice::operator==(const QInputDevice &other) const
 {
-    return id() == other.id();
+    return systemId() == other.systemId();
 }
 
 #ifndef QT_NO_DEBUG_STREAM
@@ -311,7 +315,7 @@ QDebug operator<<(QDebug debug, const QInputDevice *device)
     debug << "QInputDevice(";
     if (device) {
         debug << '"' << device->name() << "\", type=" << device->type()
-              << Qt::hex << ", ID=" << device->id() << ", seat='" << device->seatName() << "'";
+              << Qt::hex << ", ID=" << device->systemId() << ", seat='" << device->seatName() << "'";
     } else {
         debug << '0';
     }

src/gui/kernel/qinputdevice.h

@@ -56,7 +56,7 @@ class Q_GUI_EXPORT QInputDevice : public QObject
     Q_PROPERTY(QString name READ name CONSTANT)
     Q_PROPERTY(DeviceType type READ type CONSTANT)
     Q_PROPERTY(Capabilities capabilities READ capabilities CONSTANT)
-    Q_PROPERTY(qint64 id READ id CONSTANT)
+    Q_PROPERTY(qint64 systemId READ systemId CONSTANT)
     Q_PROPERTY(QString seatName READ seatName CONSTANT)
     Q_PROPERTY(QRect availableVirtualGeometry READ availableVirtualGeometry NOTIFY availableVirtualGeometryChanged)
 
@@ -98,14 +98,14 @@ public:
 
     QInputDevice();
     ~QInputDevice();
-    QInputDevice(const QString &name, qint64 id, DeviceType type,
+    QInputDevice(const QString &name, qint64 systemId, DeviceType type,
                  const QString &seatName = QString(), QObject *parent = nullptr);
 
     QString name() const;
     DeviceType type() const;
     Capabilities capabilities() const;
     bool hasCapability(Capability cap) const;
-    qint64 id() const;
+    qint64 systemId() const;
     QString seatName() const;
     QRect availableVirtualGeometry() const;
 

src/gui/kernel/qinputdevice_p.h

@@ -61,17 +61,17 @@ class Q_GUI_EXPORT QInputDevicePrivate : public QObjectPrivate
 {
     Q_DECLARE_PUBLIC(QInputDevice)
 public:
-    QInputDevicePrivate(const QString &name, qint64 id, QInputDevice::DeviceType type,
+    QInputDevicePrivate(const QString &name, qint64 winSysId, QInputDevice::DeviceType type,
                         QInputDevice::Capabilities caps = QInputDevice::Capability::None,
                         const QString &seatName = QString())
-      : name(name), seatName(seatName), id(id), capabilities(caps),
+      : name(name), seatName(seatName), systemId(winSysId), capabilities(caps),
         deviceType(type), pointingDeviceType(false)
     {
         // if the platform doesn't provide device IDs, make one up,
         // but try to avoid clashing with OS-provided 32-bit IDs
         static qint64 nextId = qint64(1) << 33;
-        if (!id)
+        if (!systemId)
-            id = nextId++;
+            systemId = nextId++;
     }
 
     QString name;
@@ -80,7 +80,7 @@ public:
     QRect availableVirtualGeometry;
     void *extra = nullptr;      // The QPA plugin can store arbitrary device-specific data here
     void *qqExtra = nullptr;    // Qt Quick can store arbitrary device-specific data here
-    qint64 id = 0;
+    qint64 systemId = 0;
     qint32 capabilities = static_cast<qint32>(QInputDevice::Capability::None);
     QInputDevice::DeviceType deviceType = QInputDevice::DeviceType::Unknown;
     qint16 pointingDeviceType : 1; // actually bool, but pack with deviceType
@@ -88,7 +88,7 @@ public:
     static void registerDevice(const QInputDevice *dev);
     static void unregisterDevice(const QInputDevice *dev);
     static bool isRegistered(const QInputDevice *dev);
-    static const QInputDevice *fromId(qint64 id);  // window system ID (e.g. xinput id), not QPointingDeviceUniqueId
+    static const QInputDevice *fromId(qint64 systemId);
 
     void setAvailableVirtualGeometry(QRect a)
     {

src/gui/kernel/qpointingdevice.cpp

@@ -345,41 +345,70 @@ const QPointingDevice *QPointingDevice::primaryPointingDevice(const QString& sea
 }
 
 /*!
+    \internal
     Finds the device instance belonging to the drawing or eraser end of a particular stylus,
-    identified by its \a deviceType, \a pointerType and \a uniqueId.  The given \a busId
+    identified by its \a deviceType, \a pointerType, \a uniqueId and \a systemId.
-    may be used to update the stored USB ID, if it was not known before.
+    Returns the device found, or \c nullptr if none was found.
+
+    If \a systemId is \c 0, it's not significant for the search.
+
+    If an instance matching the given \a deviceType and \a pointerType but with
+    only a default-constructed \c uniqueId is found, it will be assumed to be
+    the one we're looking for, and its \c uniqueId will be updated to match the
+    given \a uniqueId. This is for the benefit of any platform plugin that can
+    discover the tablet itself at startup, along with the supported stylus types,
+    but then discovers specific styli later on as they come into proximity.
 */
-const QPointingDevice *QPointingDevice::tabletDevice(QInputDevice::DeviceType deviceType,
+const QPointingDevice *QPointingDevicePrivate::queryTabletDevice(QInputDevice::DeviceType deviceType,
-                                                     QPointingDevice::PointerType pointerType,
+                                                                 QPointingDevice::PointerType pointerType,
-                                                     QPointingDeviceUniqueId uniqueId, quint32 busId)
+                                                                 QPointingDeviceUniqueId uniqueId,
+                                                                 qint64 systemId)
 {
     const auto &devices = QInputDevice::devices();
     for (const QInputDevice *dev : devices) {
-        if (dev->type() < DeviceType::Puck || dev->type() > DeviceType::Airbrush)
+        if (dev->type() < QPointingDevice::DeviceType::Puck || dev->type() > QPointingDevice::DeviceType::Airbrush)
             continue;
         const QPointingDevice *pdev = static_cast<const QPointingDevice *>(dev);
         const auto devPriv = QPointingDevicePrivate::get(pdev);
         bool uniqueIdDiscovered = (devPriv->uniqueId.numericId() == 0 && uniqueId.numericId() != 0);
         if (devPriv->deviceType == deviceType && devPriv->pointerType == pointerType &&
+                (!systemId || devPriv->systemId == systemId) &&
                 (devPriv->uniqueId == uniqueId || uniqueIdDiscovered)) {
             if (uniqueIdDiscovered) {
                 const_cast<QPointingDevicePrivate *>(devPriv)->uniqueId = uniqueId;
                 qCDebug(lcQpaInputDevices) << "discovered unique ID of tablet tool" << pdev;
             }
-            if (devPriv->busId.isEmpty() && busId) {
-                const_cast<QPointingDevicePrivate *>(devPriv)->busId = QString::number(busId, 16);
-                qCDebug(lcQpaInputDevices) << "discovered USB ID" << devPriv->busId << "of" << pdev;
-            }
             return pdev;
         }
     }
-    qCDebug(lcQpaInputDevices) << "failed to find registered tablet device" << deviceType << pointerType << Qt::hex << uniqueId.numericId()
+    return nullptr;
-                               << "The platform plugin should have provided one via "
+}
-                                  "QWindowSystemInterface::registerInputDevice(). Creating a default one for now.";
+
-    QPointingDevice *dev = new QPointingDevice(QLatin1String("fake tablet"), 2, deviceType, pointerType,
+/*!
-                                               QInputDevice::Capability::Position | QInputDevice::Capability::Pressure,
+    \internal
-                                               1, 1, QString(), uniqueId);
+    Finds the device instance belonging to the drawing or eraser end of a particular stylus,
-    QInputDevicePrivate::registerDevice(dev);
+    identified by its \a deviceType, \a pointerType and \a uniqueId. If an existing device
+    is not found, a new one is created and registered, with a warning.
+
+    This function is called from QWindowSystemInterface. Platform plugins should use
+    \l queryTabletDeviceInstance() to check whether a tablet stylus coming into proximity
+    is previously known; if not known, the plugin should create and register the stylus.
+*/
+const QPointingDevice *QPointingDevicePrivate::tabletDevice(QInputDevice::DeviceType deviceType,
+                                                            QPointingDevice::PointerType pointerType,
+                                                            QPointingDeviceUniqueId uniqueId)
+{
+    const QPointingDevice *dev = queryTabletDevice(deviceType, pointerType, uniqueId);
+    if (!dev) {
+        qCDebug(lcQpaInputDevices) << "failed to find registered tablet device"
+                                   << deviceType << pointerType << Qt::hex << uniqueId.numericId()
+                                   << "The platform plugin should have provided one via "
+                                      "QWindowSystemInterface::registerInputDevice(). Creating a default one for now.";
+        dev = new QPointingDevice(QLatin1String("fake tablet"), 2, deviceType, pointerType,
+                                                   QInputDevice::Capability::Position | QInputDevice::Capability::Pressure,
+                                                   1, 1, QString(), uniqueId, QCoreApplication::instance());
+        QInputDevicePrivate::registerDevice(dev);
+    }
     return dev;
 }
 
@@ -403,7 +432,7 @@ QDebug operator<<(QDebug debug, const QPointingDevice *device)
     if (device) {
         debug << '"' << device->name() << "\", type=";
         QtDebugUtils::formatQEnum(debug, device->type());
-        debug << ", id=" << Qt::hex << device->id() << Qt::dec << ", seat=" << device->seatName();
+        debug << ", id=" << Qt::hex << device->systemId() << Qt::dec << ", seat=" << device->seatName();
         debug << ", pointerType=";
         QtDebugUtils::formatQEnum(debug, device->pointerType());
         debug << ", capabilities=";

src/gui/kernel/qpointingdevice.h

@@ -102,7 +102,7 @@ public:
 
     QPointingDevice();
     ~QPointingDevice();
-    QPointingDevice(const QString &name, qint64 id, QInputDevice::DeviceType devType,
+    QPointingDevice(const QString &name, qint64 systemId, QInputDevice::DeviceType devType,
                     PointerType pType, Capabilities caps, int maxPoints, int buttonCount,
                     const QString &seatName = QString(),
                     QPointingDeviceUniqueId uniqueId = QPointingDeviceUniqueId(),
@@ -124,10 +124,6 @@ public:
 
     static const QPointingDevice *primaryPointingDevice(const QString& seatName = QString());
 
-    static const QPointingDevice *tabletDevice(QInputDevice::DeviceType deviceType,
-                                               QPointingDevice::PointerType pointerType,
-                                               QPointingDeviceUniqueId uniqueId, quint32 usbId = 0);
-
     bool operator==(const QPointingDevice &other) const;
 
 protected:

src/gui/kernel/qpointingdevice_p.h

@@ -90,6 +90,15 @@ public:
     {
         return static_cast<const QPointingDevicePrivate *>(QObjectPrivate::get(q));
     }
+
+    static const QPointingDevice *tabletDevice(QInputDevice::DeviceType deviceType,
+                                               QPointingDevice::PointerType pointerType,
+                                               QPointingDeviceUniqueId uniqueId);
+
+    static const QPointingDevice *queryTabletDevice(QInputDevice::DeviceType deviceType,
+                                                    QPointingDevice::PointerType pointerType,
+                                                    QPointingDeviceUniqueId uniqueId,
+                                                    qint64 systemId = 0);
 };
 
 QT_END_NAMESPACE

src/gui/kernel/qwindowsysteminterface.cpp

@@ -908,8 +908,8 @@ bool QWindowSystemInterface::handleTabletEvent(QWindow *window, ulong timestamp,
                                                qreal tangentialPressure, qreal rotation, int z, qint64 uid,
                                                Qt::KeyboardModifiers modifiers)
 {
-    const QPointingDevice *dev = QPointingDevice::tabletDevice(QInputDevice::DeviceType(device),QPointingDevice::PointerType(pointerType),
+    const QPointingDevice *dev = QPointingDevicePrivate::tabletDevice(QInputDevice::DeviceType(device),QPointingDevice::PointerType(pointerType),
-                                                               QPointingDeviceUniqueId::fromNumericId(uid));
+                                                                      QPointingDeviceUniqueId::fromNumericId(uid));
     return handleTabletEvent(window, timestamp, dev, local, global, buttons, pressure,
                       xTilt, yTilt, tangentialPressure, rotation, z, modifiers);
 }
@@ -953,9 +953,9 @@ bool QWindowSystemInterface::handleTabletEnterLeaveProximityEvent(QWindow *windo
 
 bool QWindowSystemInterface::handleTabletEnterProximityEvent(ulong timestamp, int deviceType, int pointerType, qint64 uid)
 {
-    const QPointingDevice *device = QPointingDevice::tabletDevice(QInputDevice::DeviceType(deviceType),
+    const QPointingDevice *device = QPointingDevicePrivate::tabletDevice(QInputDevice::DeviceType(deviceType),
-                                                                  QPointingDevice::PointerType(pointerType),
+                                                                         QPointingDevice::PointerType(pointerType),
-                                                                  QPointingDeviceUniqueId::fromNumericId(uid));
+                                                                         QPointingDeviceUniqueId::fromNumericId(uid));
     QWindowSystemInterfacePrivate::TabletEnterProximityEvent *e =
             new QWindowSystemInterfacePrivate::TabletEnterProximityEvent(timestamp, device);
     return QWindowSystemInterfacePrivate::handleWindowSystemEvent(e);
@@ -969,9 +969,9 @@ void QWindowSystemInterface::handleTabletEnterProximityEvent(int deviceType, int
 
 bool QWindowSystemInterface::handleTabletLeaveProximityEvent(ulong timestamp, int deviceType, int pointerType, qint64 uid)
 {
-    const QPointingDevice *device = QPointingDevice::tabletDevice(QInputDevice::DeviceType(deviceType),
+    const QPointingDevice *device = QPointingDevicePrivate::tabletDevice(QInputDevice::DeviceType(deviceType),
-                                                                  QPointingDevice::PointerType(pointerType),
+                                                                         QPointingDevice::PointerType(pointerType),
-                                                                  QPointingDeviceUniqueId::fromNumericId(uid));
+                                                                         QPointingDeviceUniqueId::fromNumericId(uid));
     QWindowSystemInterfacePrivate::TabletLeaveProximityEvent *e =
             new QWindowSystemInterfacePrivate::TabletLeaveProximityEvent(timestamp, device);
     return QWindowSystemInterfacePrivate::handleWindowSystemEvent(e);

src/plugins/platforms/xcb/qxcbconnection_xi2.cpp

@@ -202,7 +202,8 @@ static const QPointingDevice *tabletToolInstance(QPointingDevice *master, const
     }
     if (pointerTypeOverride != QPointingDevice::PointerType::Unknown)
         pointerType = pointerTypeOverride;
-    const QPointingDevice *ret = QPointingDevice::tabletDevice(devType, pointerType, QPointingDeviceUniqueId::fromNumericId(uniqueId));
+    const QPointingDevice *ret = QPointingDevicePrivate::queryTabletDevice(devType, pointerType,
+                                                                           QPointingDeviceUniqueId::fromNumericId(uniqueId), id);
     if (!ret) {
         ret = new QPointingDevice(tabletName, id, devType, pointerType, caps, 1, buttonCount,
                                   master ? master->seatName() : QString(),

tests/auto/gui/kernel/qinputdevice/tst_qinputdevice.cpp

@@ -65,17 +65,17 @@ void tst_QInputDevice::multiSeatDevices()
     QVERIFY(QInputDevicePrivate::fromId(2010));
     QVERIFY(!QInputDevicePrivate::fromId(2010)->hasCapability(QInputDevice::Capability::Scroll));
     QVERIFY(QInputDevice::primaryKeyboard());
-    QCOMPARE(QInputDevice::primaryKeyboard()->id(), 0);
+    QCOMPARE(QInputDevice::primaryKeyboard()->systemId(), qint64(1) << 33);
     QVERIFY(QPointingDevice::primaryPointingDevice());
-    QCOMPARE(QPointingDevice::primaryPointingDevice()->id(), 1);
+    QCOMPARE(QPointingDevice::primaryPointingDevice()->systemId(), 1);
     QVERIFY(QInputDevice::primaryKeyboard("seat 1"));
-    QCOMPARE(QInputDevice::primaryKeyboard("seat 1")->id(), 1000);
+    QCOMPARE(QInputDevice::primaryKeyboard("seat 1")->systemId(), 1000);
     QVERIFY(QPointingDevice::primaryPointingDevice("seat 1"));
-    QCOMPARE(QPointingDevice::primaryPointingDevice("seat 1")->id(), 1010);
+    QCOMPARE(QPointingDevice::primaryPointingDevice("seat 1")->systemId(), 1010);
     QVERIFY(QInputDevice::primaryKeyboard("seat 2"));
-    QCOMPARE(QInputDevice::primaryKeyboard("seat 2")->id(), 2000);
+    QCOMPARE(QInputDevice::primaryKeyboard("seat 2")->systemId(), 2000);
     QVERIFY(QPointingDevice::primaryPointingDevice("seat 2"));
-    QCOMPARE(QPointingDevice::primaryPointingDevice("seat 2")->id(), 2010);
+    QCOMPARE(QPointingDevice::primaryPointingDevice("seat 2")->systemId(), 2010);
 }
 
 QTEST_MAIN(tst_QInputDevice)