Re: [PATCH 7/7] ui/gtk: add clipboard support

["Marc-André Lureau" <marcandre.lureau@gmail.com>]

[-- Attachment #1: Type: text/plain, Size: 8180 bytes --]

On Fri, Feb 19, 2021 at 5:29 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   |   9 ++++
>  ui/gtk-clipboard.c | 124 +++++++++++++++++++++++++++++++++++++++++++++
>  ui/gtk.c           |   1 +
>  ui/meson.build     |   2 +-
>  4 files changed, 135 insertions(+), 1 deletion(-)
>  create mode 100644 ui/gtk-clipboard.c
>
> diff --git a/include/ui/gtk.h b/include/ui/gtk.h
> index 55319843758d..08999f8835e6 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,11 @@ 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];
> +
>      DisplayOptions *opts;
>  };
>
> @@ -208,4 +214,7 @@ QEMUGLContext
> gd_gl_area_get_current_context(DisplayChangeListener *dcl);
>  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..4a7f44b25818
> --- /dev/null
> +++ b/ui/gtk-clipboard.c
> @@ -0,0 +1,124 @@
> +/*
> + * 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 "ui/gtk.h"
> +
> +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) {
> +            if (info->types[QEMU_CLIPBOARD_TYPE_TEXT].available) {
> +                qemu_clipboard_request(info, QEMU_CLIPBOARD_TYPE_TEXT);
>

Always requesting the clipboard is a bit harsh, isn't it?

+            }
> +        }
> +        return;
> +    }
> +
> +    if (self_update) {
> +        return;
> +    }
> +
> +    if (info->types[QEMU_CLIPBOARD_TYPE_TEXT].available &&
> +        info->types[QEMU_CLIPBOARD_TYPE_TEXT].data) {
> +        gtk_clipboard_set_text(gd->gtkcb[s],
> +                               info->types[QEMU_CLIPBOARD_TYPE_TEXT].data,
> +
>  info->types[QEMU_CLIPBOARD_TYPE_TEXT].size);
> +    }
> +}
> +
> +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);
>

text might be NULL if it failed.

And you must free it.

+        break;
> +    default:
> +        break;
> +    }
> +}
> +
> +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_owner_change(GtkClipboard *clipboard,
> +                            GdkEvent *event,
> +                            gpointer data)
> +{
> +    GtkDisplayState *gd = data;
> +    QemuClipboardSelection s = gd_find_selection(gd, clipboard);
> +    QemuClipboardInfo *info;
> +
> +    info = qemu_clipboard_info_new(&gd->cbpeer, s);
> +    if (gtk_clipboard_wait_is_text_available(clipboard)) {
> +        info->types[QEMU_CLIPBOARD_TYPE_TEXT].available = true;
> +    }
>

Hmm, 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...

+
> +    qemu_clipboard_update(info);
> +    qemu_clipboard_info_put(info);
> +}
> +
> +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));
>

GDK_SELECTION_CLIPBOARD


> +    gd->gtkcb[QEMU_CLIPBOARD_SELECTION_PRIMARY] =
> +        gtk_clipboard_get(gdk_atom_intern("PRIMARY", FALSE));
>

 GDK_SELECTION_PRIMARY

+    gd->gtkcb[QEMU_CLIPBOARD_SELECTION_SECONDARY] =
> +        gtk_clipboard_get(gdk_atom_intern("SECONDARY", FALSE));
>

 GDK_SELECTION_SECONDARY


> +
> +    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);
>


Might be worth having a deinit for signals, peer registration etc, even if
nothing is hooked yet in gtk.c..

Overall, calling wait & set variants of clipboard functions makes things
quite simpler to deal with. Hopefully it can stay that way...

+}
> diff --git a/ui/gtk.c b/ui/gtk.c
> index 7b412dd4fe0b..0ae3ec20f594 100644
> --- a/ui/gtk.c
> +++ b/ui/gtk.c
> @@ -2252,6 +2252,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 a98f89b48978..3ea969a6210b 100644
> --- a/ui/meson.build
> +++ b/ui/meson.build
> @@ -64,7 +64,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-egl.c',
> 'gtk-gl-area.c'))
>    ui_modules += {'gtk' : gtk_ss}
> --
> 2.29.2
>
>
>

-- 
Marc-André Lureau

[-- Attachment #2: Type: text/html, Size: 11291 bytes --]