D26858: Provide an implementation for the tablet interface

[Vlad Zahorodnii]

zzag requested changes to this revision.
zzag added inline comments.
This revision now requires changes to proceed.

INLINE COMMENTS

> test_tablet_interface.cpp:227
> +    QCOMPARE(m_surfaces.count(), 3);
> +    for (SurfaceInterface* surface : m_surfaces) {
> +        m_tool->setCurrentSurface(surface);

Align pointers to right.

> test_tablet_interface.cpp:102-118
> +    KWayland::Client::ConnectionThread* m_connection;
> +    KWayland::Client::EventQueue* m_queue;
> +    KWayland::Client::Compositor* m_clientCompositor;
> +    KWayland::Client::Seat* m_clientSeat = nullptr;
> +
> +    QThread* m_thread;
> +    Display m_display;

Put a single space before `*`.

> display.h:328
>  
> +    TabletManagerInterface* createTabletManagerInterface(QObject* parent = nullptr);
> +

Wrong pointer alignment + missing `@since`.

> tablet_interface.cpp:53
> +                                 const QString &name, const QStringList &paths,
> +                                 QObject* parent)
> +    : QObject(parent)

Align pointers to right.

> tablet_interface.cpp:366
> +    TabletManagerInterface* const q;
> +    Display* const m_display;
> +    QHash<SeatInterface*, TabletSeatInterface*> m_seats;

Put a single space before `*`.

  Display * const m_display;

> tablet_interface.cpp:63
> +{
> +    auto *client = surface->client();
> +    const auto r = d->resourceMap().value(*client);

No `auto`.

> tablet_interface.cpp:236-237
> +    void zwp_tablet_seat_v2_bind_resource(Resource *resource) override {
> +        for (auto iface : qAsConst(m_tablets))
> +            sendTabletAdded(resource, iface);
> +

Add braces.

> tablet_interface.h:52
> +
> +    TabletSeatInterface* seat(SeatInterface* seat) const;
> +

Wrong pointer alignment.

> tablet_interface.h:121
> +
> +    wl_resource* resourceForSurface(SurfaceInterface* surface);
> +

This method lacks documentation. At first, I thought that it returns the `wl_resource` for a `wl_surface`. However, the best thing would be not to leak `wl_resource` to the public API at all.

> tablet_interface.h:126
> +    friend class TabletToolInterface;
> +    explicit TabletInterface(quint32 vendorId, quint32 productId, const QString &name, const QStringList &paths, QObject* parent);
> +    class Private;

Put a single space before `*`.

> tablet_interface.h:137-145
> +    TabletInterface* addTablet(quint32 vendorId, quint32 productId, const QString &sysname, const QString &name, const QStringList &paths);
> +    TabletToolInterface* addTool(TabletToolInterface::Type type, quint64 hardwareSerial, quint64 hardwareId, const QVector<TabletToolInterface::Capability> &capabilities);
> +
> +    TabletToolInterface* toolByHardwareId(quint64 hardwareId) const;
> +    TabletInterface* tabletByName(const QString &name) const;
> +
> +private:

Align pointers to right.

REPOSITORY
  R127 KWayland

REVISION DETAIL
  https://phabricator.kde.org/D26858

To: apol, #kwin, #frameworks, zzag
Cc: zzag, kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, bruns
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-frameworks-devel/attachments/20200123/2453e77e/attachment-0001.html>