Hi

On Thu, Mar 18, 2021 at 1:25 PM Gerd Hoffmann <kraxel@redhat.com> wrote:

> This patch adds clipboard support to the qemu gtk ui.
>
> Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
> ---
>  include/ui/gtk.h   |  10 +++
>  ui/gtk-clipboard.c | 189 +++++++++++++++++++++++++++++++++++++++++++++
>  ui/gtk.c           |   1 +
>  ui/meson.build     |   2 +-
>  4 files changed, 201 insertions(+), 1 deletion(-)
>  create mode 100644 ui/gtk-clipboard.c
>
> diff --git a/include/ui/gtk.h b/include/ui/gtk.h
> index 6e751794043f..9516670ebc87 100644
> --- a/include/ui/gtk.h
> +++ b/include/ui/gtk.h
> @@ -18,6 +18,7 @@
>  #include <gdk/gdkwayland.h>
>  #endif
>
> +#include "ui/clipboard.h"
>  #include "ui/console.h"
>  #include "ui/kbd-state.h"
>  #if defined(CONFIG_OPENGL)
> @@ -137,6 +138,12 @@ struct GtkDisplayState {
>
>      bool external_pause_update;
>
> +    QemuClipboardPeer cbpeer;
> +    QemuClipboardInfo *cbinfo[QEMU_CLIPBOARD_SELECTION__COUNT];
> +    uint32_t cbpending[QEMU_CLIPBOARD_SELECTION__COUNT];
> +    GtkClipboard *gtkcb[QEMU_CLIPBOARD_SELECTION__COUNT];
> +    bool cbowner[QEMU_CLIPBOARD_SELECTION__COUNT];
> +
>      DisplayOptions *opts;
>  };
>
> @@ -207,4 +214,7 @@ void gtk_gl_area_init(void);
>  int gd_gl_area_make_current(DisplayChangeListener *dcl,
>                              QEMUGLContext ctx);
>
> +/* gtk-clipboard.c */
> +void gd_clipboard_init(GtkDisplayState *gd);
> +
>  #endif /* UI_GTK_H */
> diff --git a/ui/gtk-clipboard.c b/ui/gtk-clipboard.c
> new file mode 100644
> index 000000000000..3585e27cab78
> --- /dev/null
> +++ b/ui/gtk-clipboard.c
> @@ -0,0 +1,189 @@
> +/*
> + * GTK UI -- clipboard support
> + *
> + * Copyright (C) 2021 Gerd Hoffmann <kraxel@redhat.com>
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation; either version 2 of the License, or
> + * (at your option) any later version.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> + * General Public License for more details.
> + *
> + * You should have received a copy of the GNU General Public License
> + * along with this program; if not, see <http://www.gnu.org/licenses/>.
> + *
> + */
> +
> +#include "qemu/osdep.h"
> +#include "qemu-common.h"
> +#include "qemu/main-loop.h"
> +
> +#include "ui/gtk.h"
> +
> +static QemuClipboardSelection gd_find_selection(GtkDisplayState *gd,
> +                                                GtkClipboard *clipboard)
> +{
> +    QemuClipboardSelection s;
> +
> +    for (s = 0; s < QEMU_CLIPBOARD_SELECTION__COUNT; s++) {
> +        if (gd->gtkcb[s] == clipboard) {
> +            return s;
> +        }
> +    }
> +    return QEMU_CLIPBOARD_SELECTION_CLIPBOARD;
> +}
> +
> +static void gd_clipboard_get_data(GtkClipboard     *clipboard,
> +                                  GtkSelectionData *selection_data,
> +                                  guint             selection_info,
> +                                  gpointer          data)
> +{
> +    GtkDisplayState *gd = data;
> +    QemuClipboardSelection s = gd_find_selection(gd, clipboard);
> +    QemuClipboardType type = QEMU_CLIPBOARD_TYPE_TEXT;
> +    QemuClipboardInfo *info = qemu_clipboard_info_get(gd->cbinfo[s]);
> +
> +    qemu_clipboard_request(info, type);
> +    while (info == gd->cbinfo[s] &&
> +           info->types[type].available &&
> +           info->types[type].data == NULL) {
> +        main_loop_wait(false);
>

Added to the list of reasons not to want running the clipboard  (or shall I
say any UI code) inside qemu.

(gtk4 is supposed to have improved that, fwiw)

+    }
> +
> +    if (info == gd->cbinfo[s] && gd->cbowner[s]) {
> +        gtk_selection_data_set_text(selection_data,
> +                                    info->types[type].data,
> +                                    info->types[type].size);
> +    } else {
> +        /* clipboard owner changed while waiting for the data */
> +    }
> +
> +    qemu_clipboard_info_put(info);
> +}
> +
> +static void gd_clipboard_clear(GtkClipboard *clipboard,
> +                               gpointer data)
> +{
> +    GtkDisplayState *gd = data;
> +    QemuClipboardSelection s = gd_find_selection(gd, clipboard);
> +
> +    gd->cbowner[s] = false;
> +}
> +
> +static void gd_clipboard_notify(Notifier *notifier, void *data)
> +{
> +    GtkDisplayState *gd = container_of(notifier, GtkDisplayState,
> cbpeer.update);
> +    QemuClipboardInfo *info = data;
> +    QemuClipboardSelection s = info->selection;
> +    bool self_update = info->owner == &gd->cbpeer;
> +
> +    if (info != gd->cbinfo[s]) {
> +        qemu_clipboard_info_put(gd->cbinfo[s]);
> +        gd->cbinfo[s] = qemu_clipboard_info_get(info);
> +        gd->cbpending[s] = 0;
> +        if (!self_update) {
> +            GtkTargetList *list;
> +            GtkTargetEntry *targets;
> +            gint n_targets;
> +
> +            list = gtk_target_list_new(NULL, 0);
> +            if (info->types[QEMU_CLIPBOARD_TYPE_TEXT].available) {
> +                gtk_target_list_add_text_targets(list, 0);
> +            }
> +            targets = gtk_target_table_new_from_list(list, &n_targets);
> +
> +            gtk_clipboard_clear(gd->gtkcb[s]);
> +            gd->cbowner[s] = true;
> +            gtk_clipboard_set_with_data(gd->gtkcb[s],
> +                                        targets, n_targets,
> +                                        gd_clipboard_get_data,
> +                                        gd_clipboard_clear,
> +                                        gd);
> +
> +            gtk_target_table_free(targets, n_targets);
> +            gtk_target_list_unref(list);
> +        }
> +        return;
> +    }
> +
> +    if (self_update) {
> +        return;
> +    }
> +
> +    /*
> +     * Clipboard got updated, with data probably.  No action here, we
> +     * are waiting for updates in gd_clipboard_get_data().
> +     */
> +}
> +
> +static void gd_clipboard_request(QemuClipboardInfo *info,
> +                                 QemuClipboardType type)
> +{
> +    GtkDisplayState *gd = container_of(info->owner, GtkDisplayState,
> cbpeer);
> +    char *text;
> +
> +    switch (type) {
> +    case QEMU_CLIPBOARD_TYPE_TEXT:
> +        text = gtk_clipboard_wait_for_text(gd->gtkcb[info->selection]);
> +        qemu_clipboard_set_data(&gd->cbpeer, info, type,
> +                                strlen(text), text, true);
>

From v1:
text might be NULL if it failed.

And you must free it.

+        break;
> +    default:
> +        break;
> +    }
> +}
> +
> +static void gd_owner_change(GtkClipboard *clipboard,
> +                            GdkEvent *event,
> +                            gpointer data)
> +{
> +    GtkDisplayState *gd = data;
> +    QemuClipboardSelection s = gd_find_selection(gd, clipboard);
> +    QemuClipboardInfo *info;
> +
> +    if (gd->cbowner[s]) {
> +        /* ignore notifications about our own grabs */
> +        return;
> +    }
> +
> +
> +    switch (event->owner_change.reason) {
> +    case GDK_SETTING_ACTION_NEW:
> +        info = qemu_clipboard_info_new(&gd->cbpeer, s);
> +        if (gtk_clipboard_wait_is_text_available(clipboard)) {
> +            info->types[QEMU_CLIPBOARD_TYPE_TEXT].available = true;
> +        }
>

Same comment as v1:
So after gtk_clipboard_set_text() the client side is actually taking
the ownership away from the guest clipboard I presume. That might have some
weird interaction issues. Hopefully the other side isn't playing the same
game...


If we don't address it now, I think it deserves a warning comment, possibly
a FIXME.

+
> +        qemu_clipboard_update(info);
> +        qemu_clipboard_info_put(info);
> +        break;
> +    default:
> +        break;
> +    }
> +}
> +
> +void gd_clipboard_init(GtkDisplayState *gd)
> +{
> +    gd->cbpeer.name = "gtk";
> +    gd->cbpeer.update.notify = gd_clipboard_notify;
> +    gd->cbpeer.request = gd_clipboard_request;
> +    qemu_clipboard_peer_register(&gd->cbpeer);
> +
> +    gd->gtkcb[QEMU_CLIPBOARD_SELECTION_CLIPBOARD] =
> +        gtk_clipboard_get(gdk_atom_intern("CLIPBOARD", FALSE));
>

Why not use GDK_SELECTION_* ?

+    gd->gtkcb[QEMU_CLIPBOARD_SELECTION_PRIMARY] =
> +        gtk_clipboard_get(gdk_atom_intern("PRIMARY", FALSE));
> +    gd->gtkcb[QEMU_CLIPBOARD_SELECTION_SECONDARY] =
> +        gtk_clipboard_get(gdk_atom_intern("SECONDARY", FALSE));
> +
> +    g_signal_connect(gd->gtkcb[QEMU_CLIPBOARD_SELECTION_CLIPBOARD],
> +                     "owner-change", G_CALLBACK(gd_owner_change), gd);
> +    g_signal_connect(gd->gtkcb[QEMU_CLIPBOARD_SELECTION_PRIMARY],
> +                     "owner-change", G_CALLBACK(gd_owner_change), gd);
> +    g_signal_connect(gd->gtkcb[QEMU_CLIPBOARD_SELECTION_SECONDARY],
> +                     "owner-change", G_CALLBACK(gd_owner_change), gd);
> +}
> diff --git a/ui/gtk.c b/ui/gtk.c
> index 7da288a25156..98046f577b9d 100644
> --- a/ui/gtk.c
> +++ b/ui/gtk.c
> @@ -2267,6 +2267,7 @@ static void gtk_display_init(DisplayState *ds,
> DisplayOptions *opts)
>          opts->u.gtk.grab_on_hover) {
>          gtk_menu_item_activate(GTK_MENU_ITEM(s->grab_on_hover_item));
>      }
> +    gd_clipboard_init(s);
>  }
>
>  static void early_gtk_display_init(DisplayOptions *opts)
> diff --git a/ui/meson.build b/ui/meson.build
> index 6c0c6ddd74eb..9ec0c03b5ad1 100644
> --- a/ui/meson.build
> +++ b/ui/meson.build
> @@ -65,7 +65,7 @@ if gtk.found()
>    softmmu_ss.add(when: 'CONFIG_WIN32', if_true: files('win32-kbd-hook.c'))
>
>    gtk_ss = ss.source_set()
> -  gtk_ss.add(gtk, vte, pixman, files('gtk.c'))
> +  gtk_ss.add(gtk, vte, pixman, files('gtk.c', 'gtk-clipboard.c'))
>    gtk_ss.add(when: x11, if_true: files('x_keymap.c'))
>    gtk_ss.add(when: [opengl, 'CONFIG_OPENGL'], if_true:
> files('gtk-gl-area.c'))
>    gtk_ss.add(when: [x11, opengl, 'CONFIG_OPENGL'], if_true:
> files('gtk-egl.c'))
> --
> 2.30.2
>
>
>

-- 
Marc-André Lureau