Hi
On Wed, Nov 8, 2023 at 7:56 PM Kamay Xutax <ad...@xutaxkamay.com> wrote:
>
> Signed-off-by: Kamay Xutax <ad...@xutaxkamay.com>
> ---
> include/ui/sdl2.h | 5 ++
> meson.build | 1 +
> ui/meson.build | 1 +
> ui/sdl2-clipboard.c | 147 ++++++++++++++++++++++++++++++++++++++++++++
> ui/sdl2.c | 8 +++
> 5 files changed, 162 insertions(+)
> create mode 100644 ui/sdl2-clipboard.c
>
> diff --git a/include/ui/sdl2.h b/include/ui/sdl2.h
> index e3acc7c82a..120fe6f856 100644
> --- a/include/ui/sdl2.h
> +++ b/include/ui/sdl2.h
> @@ -21,6 +21,7 @@
> # include <SDL_image.h>
> #endif
>
> +#include "ui/clipboard.h"
> #include "ui/kbd-state.h"
> #ifdef CONFIG_OPENGL
> # include "ui/egl-helpers.h"
> @@ -51,6 +52,7 @@ struct sdl2_console {
> bool y0_top;
> bool scanout_mode;
> #endif
> + QemuClipboardPeer cbpeer;
> };
>
> void sdl2_window_create(struct sdl2_console *scon);
> @@ -70,6 +72,9 @@ void sdl2_2d_redraw(struct sdl2_console *scon);
> bool sdl2_2d_check_format(DisplayChangeListener *dcl,
> pixman_format_code_t format);
>
> +void sdl2_clipboard_handle_request(struct sdl2_console *scon);
> +void sdl2_clipboard_init(struct sdl2_console *scon);
> +
> void sdl2_gl_update(DisplayChangeListener *dcl,
> int x, int y, int w, int h);
> void sdl2_gl_switch(DisplayChangeListener *dcl,
> diff --git a/meson.build b/meson.build
> index 4848930680..1358d14b2e 100644
> --- a/meson.build
> +++ b/meson.build
> @@ -2157,6 +2157,7 @@ config_host_data.set('CONFIG_RDMA', rdma.found())
> config_host_data.set('CONFIG_RELOCATABLE', get_option('relocatable'))
> config_host_data.set('CONFIG_SAFESTACK', get_option('safe_stack'))
> config_host_data.set('CONFIG_SDL', sdl.found())
> +config_host_data.set('CONFIG_SDL_CLIPBOARD', sdl.found())
'gtk_clipboard' option is there because it has some issues with the
glib loop - see https://gitlab.com/qemu-project/qemu/-/issues/1150.
Apparently this code could have similar kind of issues, since it's
reentering the main loop too. it might be worth to have a similar
option and disclaimer...
> config_host_data.set('CONFIG_SDL_IMAGE', sdl_image.found())
> config_host_data.set('CONFIG_SECCOMP', seccomp.found())
> if seccomp.found()
> diff --git a/ui/meson.build b/ui/meson.build
> index 0ccb3387ee..0cadd1a18f 100644
> --- a/ui/meson.build
> +++ b/ui/meson.build
> @@ -125,6 +125,7 @@ if sdl.found()
> sdl_ss = ss.source_set()
> sdl_ss.add(sdl, sdl_image, pixman, glib, files(
> 'sdl2-2d.c',
> + 'sdl2-clipboard.c',
> 'sdl2-input.c',
> 'sdl2.c',
> ))
> diff --git a/ui/sdl2-clipboard.c b/ui/sdl2-clipboard.c
> new file mode 100644
> index 0000000000..405bb9ea8b
> --- /dev/null
> +++ b/ui/sdl2-clipboard.c
> @@ -0,0 +1,147 @@
> +/*
> + * SDL UI -- clipboard support
> + *
> + * Copyright (C) 2023 Kamay Xutax <ad...@xutaxkamay.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/main-loop.h"
> +#include "ui/sdl2.h"
> +
> +static void sdl2_clipboard_update(struct sdl2_console *scon,
> + QemuClipboardInfo *info)
> +{
> + bool self_update = info->owner == &scon->cbpeer;
> + char *text;
> + size_t text_size;
> +
> + /*
> + * In case of a self update,
> + * set again the text in SDL
> + *
> + * This is a workaround for hosts that have clipboard history
> + * or when they're copying again something,
> + * so that SDL can accept a new request from the host
> + * and make a new SDL_CLIPBOARDUPDATE event
> + */
> +
> + if (self_update) {
> + text = SDL_GetClipboardText();
> + SDL_SetClipboardText(text);
> + SDL_free(text);
> + return;
> + }
Isn't this basically doing the work of a clipboard manager? it takes
the current clipboard data and makes qemu the new owner. It looks like
it could also run in a loop quite easily if it fights with a manager.
> +
> + if (!info->types[QEMU_CLIPBOARD_TYPE_TEXT].available) {
> + return;
> + }
> +
> + info = qemu_clipboard_info_ref(info);
> + qemu_clipboard_request(info, QEMU_CLIPBOARD_TYPE_TEXT);
> +
> + while (info == qemu_clipboard_info(info->selection) &&
> + info->types[QEMU_CLIPBOARD_TYPE_TEXT].available &&
> + info->types[QEMU_CLIPBOARD_TYPE_TEXT].data == NULL) {
> + main_loop_wait(false);
Reentering the loop, that's annoying... same as gtk-clipboard.c..
Have you tried to defer the handling of the update? That will add
extra state & logic though.
> + }
> +
> + /* clipboard info changed while waiting for data */
> + if (info != qemu_clipboard_info(info->selection)) {
> + qemu_clipboard_info_unref(info);
> + return;
> + }
> +
> + /* text is not null terminated in cb info, so we need to copy it */
> + text_size = info->types[QEMU_CLIPBOARD_TYPE_TEXT].size;
hmm, I wonder why.. isn't it something we could fix from
qemu_clipboard_set_data() callers?
(gtk_selection_data_set_text() doc says it should be \0 terminated,
although it seems not required when len is given).
I am not sure if the spice/vdagent ensures \0 terminated strings, but
the qemu code could adjust it as necessary.
> + if (!text_size) {
> + qemu_clipboard_info_unref(info);
> + return;
> + }
> +
> + text = malloc(text_size + 1);
We use g_malloc() and g_free() (even better with g_autofree).
> +
> + if (!text) {
Then, no need to check for NULL results (it aborts on OOM).
> + qemu_clipboard_info_unref(info);
> + return;
> + }
> +
> + text[text_size] = 0;
> + memcpy(text, info->types[QEMU_CLIPBOARD_TYPE_TEXT].data, text_size);
> + /* unref as soon we copied the text */
> + qemu_clipboard_info_unref(info);
> + SDL_SetClipboardText(text);
> +
> + free(text);
> +}
> +
> +static void sdl2_clipboard_notify(Notifier *notifier,
> + void *data)
> +{
> + QemuClipboardNotify *notify = data;
> + struct sdl2_console *scon =
> + container_of(notifier, struct sdl2_console, cbpeer.notifier);
> +
> + switch (notify->type) {
> + case QEMU_CLIPBOARD_UPDATE_INFO:
> + sdl2_clipboard_update(scon, notify->info);
> + break;
> + case QEMU_CLIPBOARD_RESET_SERIAL:
> + break;
> + }
> +}
> +
> +static void sdl2_clipboard_request(QemuClipboardInfo *info,
> + QemuClipboardType type)
> +{
> + struct sdl2_console *scon =
> + container_of(info->owner, struct sdl2_console, cbpeer);
> + char *text;
> +
> + switch (type) {
> + case QEMU_CLIPBOARD_TYPE_TEXT:
> + if (!SDL_HasClipboardText()) {
> + return;
> + }
> +
> + text = SDL_GetClipboardText();
> + qemu_clipboard_set_data(&scon->cbpeer, info, type,
> + strlen(text), text, true);
strlen() + 1 to have \0 then? (other backends would need similar fix)
> +
> + SDL_free(text);
> + break;
> + default:
> + return;
> + }
> +}
> +
> +void sdl2_clipboard_handle_request(struct sdl2_console *scon)
> +{
> + g_autoptr(QemuClipboardInfo) info =
> + qemu_clipboard_info_new(&scon->cbpeer,
> + QEMU_CLIPBOARD_SELECTION_CLIPBOARD);
> +
> + sdl2_clipboard_request(info, QEMU_CLIPBOARD_TYPE_TEXT);
> +}
> +
> +void sdl2_clipboard_init(struct sdl2_console *scon)
> +{
> + scon->cbpeer.name = "sdl2";
> + scon->cbpeer.notifier.notify = sdl2_clipboard_notify;
> + /* requests will be handled from the SDL event loop */
> + qemu_clipboard_peer_register(&scon->cbpeer);
> +}
> diff --git a/ui/sdl2.c b/ui/sdl2.c
> index fbfdb64e90..d674add7c5 100644
> --- a/ui/sdl2.c
> +++ b/ui/sdl2.c
> @@ -702,6 +702,11 @@ void sdl2_poll_events(struct sdl2_console *scon)
> case SDL_WINDOWEVENT:
> handle_windowevent(ev);
> break;
> +#if defined(CONFIG_SDL_CLIPBOARD)
> + case SDL_CLIPBOARDUPDATE:
> + sdl2_clipboard_handle_request(scon);
> + break;
> +#endif
> default:
> break;
> }
> @@ -922,6 +927,9 @@ static void sdl2_display_init(DisplayState *ds,
> DisplayOptions *o)
> qemu_console_set_window_id(con, info.info.x11.window);
> #endif
> }
> +#endif
> +#if defined(CONFIG_SDL_CLIPBOARD)
> + sdl2_clipboard_init(&sdl2_console[i]);
> #endif
> }
>
> --
> 2.41.0
>
>
--
Marc-André Lureau
