Introduce DragAndDropIcon

Every time the icon surface is committed, we need to accumulate the
surface offset. However, this is not easy to do on the compositor side
with the current design.

This change introduces a new class DragAndDropIcon that defines a
surface role as well as accumulates surface offsets on every commit.

This fixes a bug where the dnd icon jumps all of sudden while dragging
an image from Firefox to other applications.

src/wayland/autotests/client/test_datadevice.cpp

@@ -352,7 +352,12 @@ void TestDataDevice::testDragInternally()
     QCOMPARE(!dragStartedSpy.isEmpty(), success);
     QVERIFY(!deviceInterface->dragSource());
     QCOMPARE(deviceInterface->origin(), success ? surfaceInterface : nullptr);
-    QCOMPARE(deviceInterface->icon(), success ? iconSurfaceInterface : nullptr);
+
+    if (success) {
+        QCOMPARE(deviceInterface->icon()->surface(), iconSurfaceInterface);
+    } else {
+        QCOMPARE(deviceInterface->icon(), nullptr);
+    }
 }
 
 void TestDataDevice::testSetSelection()

src/wayland/datadevice_interface.cpp

@@ -1,58 +1,70 @@
 /*
     SPDX-FileCopyrightText: 2014 Martin Gräßlin <mgraesslin@kde.org>
     SPDX-FileCopyrightText: 2020 David Edmundson <davidedmundson@kde.org>
+    SPDX-FileCopyrightText: 2020 Vlad Zahorodnii <vlad.zahorodnii@kde.org>
 
     SPDX-License-Identifier: LGPL-2.1-only OR LGPL-3.0-only OR LicenseRef-KDE-Accepted-LGPL
 */
 #include "datadevice_interface.h"
+#include "datadevice_interface_p.h"
 #include "datadevicemanager_interface.h"
 #include "datasource_interface.h"
 #include "dataoffer_interface.h"
 #include "display.h"
-#include "resource_p.h"
 #include "pointer_interface.h"
 #include "seat_interface.h"
 #include "seat_interface_p.h"
 #include "surface_interface.h"
-#include <qwayland-server-wayland.h>
+#include "surfacerole_p.h"
 
 namespace KWaylandServer
 {
 
-class DataDeviceInterfacePrivate : public QtWaylandServer::wl_data_device
+class DragAndDropIconPrivate : public SurfaceRole
 {
 public:
-    DataDeviceInterfacePrivate(SeatInterface *seat, DataDeviceInterface *_q, wl_resource *resource);
+    explicit DragAndDropIconPrivate(SurfaceInterface *surface);
 
-    DataOfferInterface *createDataOffer(AbstractDataSource *source);
+    void commit() override;
 
-    SeatInterface *seat;
-    DataSourceInterface *source = nullptr;
-    SurfaceInterface *surface = nullptr;
-    QPointer<SurfaceInterface> icon;
-    QPointer<DataSourceInterface> selection;
-
-    struct Drag {
-        SurfaceInterface *surface = nullptr;
-        QMetaObject::Connection destroyConnection;
-        QMetaObject::Connection posConnection;
-        QMetaObject::Connection sourceActionConnection;
-        QMetaObject::Connection targetActionConnection;
-        quint32 serial = 0;
-    };
-    Drag drag;
-
-    DataDeviceInterface *q;
-
-    QPointer<SurfaceInterface> proxyRemoteSurface;
-
-protected:
-    void data_device_destroy_resource(Resource *resource) override;
-    void data_device_start_drag(Resource *resource, wl_resource *source, wl_resource *origin, wl_resource *icon, uint32_t serial) override;
-    void data_device_set_selection(Resource *resource, wl_resource *source, uint32_t serial) override;
-    void data_device_release(Resource *resource) override;
+    QPoint position;
 };
 
+DragAndDropIconPrivate::DragAndDropIconPrivate(SurfaceInterface *surface)
+    : SurfaceRole(surface, QByteArrayLiteral("dnd_icon"))
+{
+}
+
+void DragAndDropIconPrivate::commit()
+{
+    position += surface()->offset();
+}
+
+DragAndDropIcon::DragAndDropIcon(SurfaceInterface *surface, QObject *parent)
+    : QObject(parent)
+    , d(new DragAndDropIconPrivate(surface))
+{
+}
+
+DragAndDropIcon::~DragAndDropIcon()
+{
+}
+
+QPoint DragAndDropIcon::position() const
+{
+    return d->position;
+}
+
+SurfaceInterface *DragAndDropIcon::surface() const
+{
+    return d->surface();
+}
+
+DataDeviceInterfacePrivate *DataDeviceInterfacePrivate::get(DataDeviceInterface *device)
+{
+    return device->d.data();
+}
+
 DataDeviceInterfacePrivate::DataDeviceInterfacePrivate(SeatInterface *seat, DataDeviceInterface *_q, wl_resource *resource)
     : QtWaylandServer::wl_data_device(resource)
     , seat(seat)
@@ -60,11 +72,24 @@ DataDeviceInterfacePrivate::DataDeviceInterfacePrivate(SeatInterface *seat, Data
 {
 }
 
+void DataDeviceInterfacePrivate::endDrag()
+{
+    icon.reset();
+}
+
 void DataDeviceInterfacePrivate::data_device_start_drag(Resource *resource, wl_resource *sourceResource, wl_resource *originResource, wl_resource *iconResource, uint32_t serial)
 {
-    Q_UNUSED(resource)
+    SurfaceInterface *iconSurface = SurfaceInterface::get(iconResource);
+
+    const SurfaceRole *surfaceRole = SurfaceRole::get(iconSurface);
+    if (surfaceRole) {
+        wl_resource_post_error(resource->handle, error_role,
+                               "the icon surface already has a role assigned %s",
+                               surfaceRole->name().constData());
+        return;
+    }
+
     SurfaceInterface *focusSurface = SurfaceInterface::get(originResource);
-    SurfaceInterface *i = SurfaceInterface::get(iconResource);
     DataSourceInterface *dataSource = nullptr;
     if (sourceResource) {
         dataSource = DataSourceInterface::get(sourceResource);
@@ -88,8 +113,11 @@ void DataDeviceInterfacePrivate::data_device_start_drag(Resource *resource, wl_r
     if (dataSource) {
         QObject::connect(dataSource, &AbstractDataSource::aboutToBeDestroyed, q, [this] { source = nullptr; });
     }
+    if (iconSurface) {
+        icon.reset(new DragAndDropIcon(iconSurface));
+        QObject::connect(iconSurface, &SurfaceInterface::aboutToBeDestroyed, icon.data(), [this] { icon.reset(); });
+    }
     surface = focusSurface;
-    icon = i;
     drag.serial = serial;
     emit q->dragStarted();
 }
@@ -169,9 +197,9 @@ DataSourceInterface *DataDeviceInterface::dragSource() const
     return d->source;
 }
 
-SurfaceInterface *DataDeviceInterface::icon() const
+DragAndDropIcon *DataDeviceInterface::icon() const
 {
-    return d->icon;
+    return d->icon.data();
 }
 
 SurfaceInterface *DataDeviceInterface::origin() const

src/wayland/datadevice_interface.h

@@ -1,6 +1,7 @@
 /*
     SPDX-FileCopyrightText: 2014 Martin Gräßlin <mgraesslin@kde.org>
     SPDX-FileCopyrightText: 2020 David Edmundson <davidedmundson@kde.org>
+    SPDX-FileCopyrightText: 2020 Vlad Zahorodnii <vlad.zahorodnii@kde.org>
 
     SPDX-License-Identifier: LGPL-2.1-only OR LGPL-3.0-only OR LicenseRef-KDE-Accepted-LGPL
 */
@@ -23,6 +24,36 @@ class AbstractDataSource;
 class SeatInterface;
 class SurfaceInterface;
 class DataDeviceInterfacePrivate;
+class DragAndDropIconPrivate;
+
+/**
+ * The DragAndDropIcon class represents a drag-and-drop icon.
+ *
+ * Note that the lifetime of the drag-and-drop icon is bound to the lifetime of the underlying
+ * icon surface.
+ */
+class KWAYLANDSERVER_EXPORT DragAndDropIcon : public QObject
+{
+    Q_OBJECT
+
+public:
+    ~DragAndDropIcon() override;
+
+    /**
+     * Returns the position of the icon relative to the cursor's hotspot.
+     */
+    QPoint position() const;
+
+    /**
+     * Returns the underlying icon surface. This function always returns a valid surface.
+     */
+    SurfaceInterface *surface() const;
+
+private:
+    explicit DragAndDropIcon(SurfaceInterface *surface, QObject *parent = nullptr);
+    friend class DataDeviceInterfacePrivate;
+    QScopedPointer<DragAndDropIconPrivate> d;
+};
 
 /**
  * @brief DataDeviceInterface allows clients to share data by copy-and-paste and drag-and-drop.
@@ -44,7 +75,11 @@ public:
     SeatInterface *seat() const;
     DataSourceInterface *dragSource() const;
     SurfaceInterface *origin() const;
-    SurfaceInterface *icon() const;
+    /**
+     * Returns the additional icon attached to the cursor during a drag-and-drop operation.
+     * This function returns @c null if no drag-and-drop icon has been attached.
+     */
+    DragAndDropIcon *icon() const;
 
     /**
      * @returns the serial of the implicit grab which started the drag
@@ -91,6 +126,7 @@ private:
     friend class DataDeviceManagerInterfacePrivate;
     explicit DataDeviceInterface(SeatInterface *seat, wl_resource *resource);
     QScopedPointer<DataDeviceInterfacePrivate> d;
+    friend class DataDeviceInterfacePrivate;
 };
 
 }

src/wayland/datadevice_interface_p.h

@@ -0,0 +1,60 @@
+/*
+    SPDX-FileCopyrightText: 2014 Martin Gräßlin <mgraesslin@kde.org>
+    SPDX-FileCopyrightText: 2020 David Edmundson <davidedmundson@kde.org>
+
+    SPDX-License-Identifier: LGPL-2.1-only OR LGPL-3.0-only OR LicenseRef-KDE-Accepted-LGPL
+*/
+
+#pragma once
+
+#include <QPointer>
+
+#include "qwayland-server-wayland.h"
+
+namespace KWaylandServer
+{
+
+class AbstractDataSource;
+class DataDeviceInterface;
+class DataOfferInterface;
+class DataSourceInterface;
+class DragAndDropIcon;
+class SeatInterface;
+class SurfaceInterface;
+
+class DataDeviceInterfacePrivate : public QtWaylandServer::wl_data_device
+{
+public:
+    static DataDeviceInterfacePrivate *get(DataDeviceInterface *device);
+
+    DataDeviceInterfacePrivate(SeatInterface *seat, DataDeviceInterface *_q, wl_resource *resource);
+
+    DataOfferInterface *createDataOffer(AbstractDataSource *source);
+    void endDrag();
+
+    SeatInterface *seat;
+    DataDeviceInterface *q;
+    DataSourceInterface *source = nullptr;
+    SurfaceInterface *surface = nullptr;
+    QScopedPointer<DragAndDropIcon> icon;
+    QPointer<DataSourceInterface> selection;
+    QPointer<SurfaceInterface> proxyRemoteSurface;
+
+    struct Drag {
+        SurfaceInterface *surface = nullptr;
+        QMetaObject::Connection destroyConnection;
+        QMetaObject::Connection posConnection;
+        QMetaObject::Connection sourceActionConnection;
+        QMetaObject::Connection targetActionConnection;
+        quint32 serial = 0;
+    };
+    Drag drag;
+
+protected:
+    void data_device_destroy_resource(Resource *resource) override;
+    void data_device_start_drag(Resource *resource, wl_resource *source, wl_resource *origin, wl_resource *icon, uint32_t serial) override;
+    void data_device_set_selection(Resource *resource, wl_resource *source, uint32_t serial) override;
+    void data_device_release(Resource *resource) override;
+};
+
+} // namespace KWaylandServer

src/wayland/seat_interface.cpp

@@ -9,6 +9,7 @@
 #include "seat_interface_p.h"
 #include "display.h"
 #include "datadevice_interface.h"
+#include "datadevice_interface_p.h"
 #include "datasource_interface.h"
 #include "datacontroldevice_v1_interface.h"
 #include "datacontrolsource_v1_interface.h"
@@ -399,12 +400,13 @@ void SeatInterface::Private::endDrag(quint32 serial)
     QObject::disconnect(drag.destroyConnection);
     QObject::disconnect(drag.dragSourceDestroyConnection);
 
-    DataDeviceInterface *dragTarget = drag.target;
-    DataSourceInterface *dragSource = drag.source ? drag.source->dragSource() : nullptr;
+    DataDeviceInterface *dragTargetDevice = drag.target;
+    DataDeviceInterface *dragSourceDevice = drag.source;
+    DataSourceInterface *dragSource = dragSourceDevice ? dragSourceDevice->dragSource() : nullptr;
     if (dragSource) {
         // TODO: Also check the current drag-and-drop action.
-        if (dragTarget && dragSource->isAccepted()) {
-            dragTarget->drop();
+        if (dragTargetDevice && dragSource->isAccepted()) {
+            dragTargetDevice->drop();
             dragSource->dropPerformed();
         } else {
             if (wl_resource_get_version(dragSource->resource()) >=
@@ -414,8 +416,12 @@ void SeatInterface::Private::endDrag(quint32 serial)
         }
     }
 
-    if (dragTarget) {
-        dragTarget->updateDragTarget(nullptr, serial);
+    if (dragTargetDevice) {
+        dragTargetDevice->updateDragTarget(nullptr, serial);
+    }
+    if (dragSourceDevice) {
+        auto devicePrivate = DataDeviceInterfacePrivate::get(dragSourceDevice);
+        devicePrivate->endDrag();
     }
 
     drag = Drag();