[Spice-devel] [PATCH] client: support clipboard/selection-owner model
Hans de Goede
hdegoede at redhat.com
Thu Sep 23 08:06:33 PDT 2010
Hi,
See comments inline.
On 09/22/2010 02:16 PM, Arnon Gilboa wrote:
> -support the GRAB/REQUEST/DATA/RELEASE verbs in both ways
> -pasting clipboard data is now "only-by-demand" from both sides (client and agent), whose behavior is symmetric
> -client and agent don't read or send the contents of the clipboard unnecessarily (e.g. copy, internal paste, repeating paste, focus change)
> -set client as clipboard listener instead of application
> -add atexit(cleanup) in win platform
>
> linux:
> -enable USE_XRANDR_1_2 and support clipboard in MultyMonScreen
> -send utf8 with no null termination, remove ++size
> -add xfixes in configure.ac& Makefile.am
>
> windows:
> -bonus: support image cut& paste, currently only on windows
> ---
> client/application.cpp | 6 -
> client/application.h | 2 -
> client/platform.h | 29 ++++-
> client/red_client.cpp | 205 +++++++++++++++++++--------
> client/red_client.h | 34 ++++-
> client/windows/platform.cpp | 240 +++++++++++++++++++++++++-------
> client/x11/Makefile.am | 1 +
> client/x11/platform.cpp | 326 +++++++++++++++++++++++++++----------------
> configure.ac | 3 +-
> 9 files changed, 597 insertions(+), 249 deletions(-)
>
> diff --git a/client/application.cpp b/client/application.cpp
> index 490cd8c..a13d5e8 100644
> --- a/client/application.cpp
> +++ b/client/application.cpp
> @@ -1325,12 +1325,6 @@ void Application::on_app_activated()
> {
> _active = true;
> _key_handler->on_focus_in();
> - Platform::set_clipboard_listener(this);
> -}
> -
> -void Application::on_clipboard_change()
> -{
> - _client.on_clipboard_change();
> }
>
> void Application::on_app_deactivated()
> diff --git a/client/application.h b/client/application.h
> index dde37fc..36ae86e 100644
> --- a/client/application.h
> +++ b/client/application.h
> @@ -141,7 +141,6 @@ typedef std::list<GUIBarrier*> GUIBarriers;
> class Application : public ProcessLoop,
> public Platform::EventListener,
> public Platform::DisplayModeListener,
> - public Platform::ClipboardListener,
> public CommandTarget {
> public:
>
> @@ -191,7 +190,6 @@ public:
> virtual void on_app_deactivated();
> virtual void on_monitors_change();
> virtual void on_display_mode_change();
> - virtual void on_clipboard_change();
> void on_connected();
> void on_disconnected(int spice_error_code);
> void on_disconnecting();
> diff --git a/client/platform.h b/client/platform.h
> index 6288380..e68ba8f 100644
> --- a/client/platform.h
> +++ b/client/platform.h
> @@ -123,11 +123,32 @@ public:
>
> enum {
> CLIPBOARD_UTF8_TEXT = 1,
> + CLIPBOARD_BITMAP
> };
>
1) I see no need for platform.h to contain a duplicate
enum with vd_agent.h we should simply use the
VD_AGENT_CLIPBOARD_* types in the platform
code too, this way we can get rid of this enum,
the ClipboardType type, the clipboard_types
array and the RedClient::get_platform_clipboard_type
and RedClient::get_agent_clipboard_type methods.
Moreover removing this will hopefully also ease
the headache I'm getting when reviewing this and
trying to figure out how the type gets transformed
from one into the other as calls are made from
one object to the next, if all conversions are done
when needed, etc.
2) I believe we should not add bitmap support until we've a
platform independent way of doing this. Adding features prematurely
only leads to troubles down the road later (see the defunct before
even really used old clipboard capability flag for example).
> + typedef struct ClipboardFormat {
> + uint32_t format;
> + uint32_t type;
> + } ClipboardFormat;
> +
> + static ClipboardFormat clipboard_formats[];
> +
> + static uint32_t get_clipboard_type(uint32_t format) {
> + ClipboardFormat* iter;
> + for (iter = clipboard_formats; iter->type&& iter->format != format; iter++);
> + return iter->type;
> + }
> +
> + static uint32_t get_clipboard_format(uint32_t type) {
> + ClipboardFormat* iter;
> + for (iter = clipboard_formats; iter->format&& iter->type != type; iter++);
> + return iter->format;
> + }
> +
Pretty much the same as above, I see no reason for anything outside the
platform specific to need the mapping between the VD_AGENT_CLIPBOARD_*
types and platform specific formats. The lower this mapping is handled
the cleaner and easier to read the remaining code is. pushing this down
will even make the platform specific code much clearer as a direct conversion
there is a lot easier to follow when reading the code then having to
trace the flow through some unneeded abstraction layer.
Last, but not least, this puts the assumption in the platform independent
code that all platform clipboards formats are stored as an uint32_t, which
seems wrong to me. For example I'm pretty sure some platform use mime types
stores as strings.
> + static bool set_clipboard_owner(uint32_t type);
> static bool set_clipboard_data(uint32_t type, const uint8_t* data, int32_t size);
> - static bool get_clipboard_data(uint32_t type, uint8_t* data, int32_t size);
> - static int32_t get_clipboard_data_size(uint32_t type);
> + static bool request_clipboard_notification(uint32_t type);
> + static void release_clipboard();
> };
>
> class Platform::EventListener {
> @@ -141,7 +162,9 @@ public:
> class Platform::ClipboardListener {
> public:
> virtual ~ClipboardListener() {}
> - virtual void on_clipboard_change() = 0;
> + virtual void on_clipboard_change(uint32_t type) = 0;
> + virtual void on_clipboard_render(uint32_t type) = 0;
> + virtual void on_clipboard_notify(uint32_t type, uint8_t* data, int32_t size) = 0;
These names seem a bit windows centric, esp. the render name, why not
simply name them after the vdagent messages. so change -> grab and
render -> request ?
> };
>
> class Platform::RecordClient {
> diff --git a/client/red_client.cpp b/client/red_client.cpp
> index 79f5e6d..ba1cb89 100644
> --- a/client/red_client.cpp
> +++ b/client/red_client.cpp
> @@ -81,9 +81,28 @@ uint32_t default_agent_caps[] = {
> (1<< VD_AGENT_CAP_REPLY)
> };
>
> -void ClipboardEvent::response(AbstractProcessLoop& events_loop)
> +typedef struct ClipboardType {
> + uint32_t agent_type;
> + uint32_t platform_type;
> +} ClipboardType;
> +
> +ClipboardType clipboard_types[] = {
> + {VD_AGENT_CLIPBOARD_UTF8_TEXT, Platform::CLIPBOARD_UTF8_TEXT},
> + {VD_AGENT_CLIPBOARD_BITMAP, Platform::CLIPBOARD_BITMAP},
> + {0, 0}};
> +
As said above this should all be removed IMHO.
> +void ClipboardGrabEvent::response(AbstractProcessLoop& events_loop)
> +{
> + VDAgentClipboardGrab grab = {_type};
> + static_cast<RedClient*>(events_loop.get_owner())->send_agent_clipboard_message(
> + VD_AGENT_CLIPBOARD_GRAB, sizeof(grab),&grab);
> +}
> +
> +void ClipboardRequestEvent::response(AbstractProcessLoop& events_loop)
> {
> - static_cast<RedClient*>(events_loop.get_owner())->send_agent_clipboard();
> + VDAgentClipboardRequest request = {_type};
> + static_cast<RedClient*>(events_loop.get_owner())->send_agent_clipboard_message(
> + VD_AGENT_CLIPBOARD_REQUEST, sizeof(request),&request);
> }
>
> Migrate::Migrate(RedClient& client)
> @@ -339,6 +358,7 @@ RedClient::RedClient(Application& application)
> , _migrate (*this)
> , _glz_window (0, _glz_debug)
> {
> + Platform::set_clipboard_listener(this);
> MainChannelLoop* message_loop = static_cast<MainChannelLoop*>(get_message_handler());
> uint32_t default_caps_size = SPICE_N_ELEMENTS(default_agent_caps);
>
> @@ -542,6 +562,7 @@ bool RedClient::abort_channels()
> bool RedClient::abort()
> {
> if (!_aborting) {
> + Platform::set_clipboard_listener(NULL);
> Lock lock(_sync_lock);
> _aborting = true;
> _sync_condition.notify_all();
Not related to this patch, but I assume the
Lock lock(_sync_lock); is some sort of locking mechanism to protect
the abort handler against concurrent access. If that is the case,
the locking is done too late as if this code is re-entrant then the test
and set of the _aborting flag needs to be protected by a lock as well.
> @@ -784,16 +805,6 @@ void RedClient::on_display_mode_change()
> #endif
> }
>
> -uint32_t get_agent_clipboard_type(uint32_t type)
> -{
> - switch (type) {
> - case Platform::CLIPBOARD_UTF8_TEXT:
> - return VD_AGENT_CLIPBOARD_UTF8_TEXT;
> - default:
> - return 0;
> - }
> -}
> -
> void RedClient::do_send_agent_clipboard()
> {
> uint32_t size;
> @@ -802,8 +813,7 @@ void RedClient::do_send_agent_clipboard()
> (size = MIN(VD_AGENT_MAX_DATA_SIZE,
> _agent_out_msg_size - _agent_out_msg_pos))) {
> Message* message = new Message(SPICE_MSGC_MAIN_AGENT_DATA);
> - void* data =
> - spice_marshaller_reserve_space(message->marshaller(), size);
> + void* data = spice_marshaller_reserve_space(message->marshaller(), size);
> memcpy(data, (uint8_t*)_agent_out_msg + _agent_out_msg_pos, size);
> _agent_tokens--;
> post_message(message);
> @@ -817,14 +827,39 @@ void RedClient::do_send_agent_clipboard()
> }
> }
>
> -//FIXME: currently supports text only; better name - poll_clipboard?
> -void RedClient::send_agent_clipboard()
> +void RedClient::send_agent_clipboard_message(uint32_t message_type, uint32_t size, void* data)
> +{
> + Message* message = new Message(SPICE_MSGC_MAIN_AGENT_DATA);
> + VDAgentMessage* msg = (VDAgentMessage*)
> + spice_marshaller_reserve_space(message->marshaller(), sizeof(VDAgentMessage) + size);
> + msg->protocol = VD_AGENT_PROTOCOL;
> + msg->type = message_type;
> + msg->opaque = 0;
> + msg->size = size;
> + if (size&& data) {
> + memcpy(msg->data, data, size);
> + }
> + ASSERT(_agent_tokens)
> + _agent_tokens--;
> + post_message(message);
> +}
> +
## begin these ###
> +void RedClient::on_clipboard_change(uint32_t type)
> {
> - //FIXME: check connected - assert on disconnect
> - uint32_t clip_type = Platform::CLIPBOARD_UTF8_TEXT;
> - int32_t clip_size = Platform::get_clipboard_data_size(clip_type);
> + AutoRef<ClipboardGrabEvent> event(new ClipboardGrabEvent(type));
> + get_process_loop().push_event(*event);
> +}
> +
> +void RedClient::on_clipboard_render(uint32_t type)
> +{
> + AutoRef<ClipboardRequestEvent> event(new ClipboardRequestEvent(type));
> + get_process_loop().push_event(*event);
> +}
>
Notice the naming issue here again clipboard_change -> ClipboardGrabEvent,
clipboard_render -> ClipboardRequestEvent.
> - if (!clip_size || !_agent_connected) {
> +void RedClient::on_clipboard_notify(uint32_t type, uint8_t* data, int32_t size)
> +{
> + ASSERT(size&& data);
> + if (!_agent_connected) {
> return;
> }
> if (_agent_out_msg) {
> @@ -832,32 +867,20 @@ void RedClient::send_agent_clipboard()
> return;
> }
> _agent_out_msg_pos = 0;
> - _agent_out_msg_size = sizeof(VDAgentMessage) + sizeof(VDAgentClipboard) + clip_size;
> + _agent_out_msg_size = sizeof(VDAgentMessage) + sizeof(VDAgentClipboard) + size;
> _agent_out_msg = (VDAgentMessage*)new uint8_t[_agent_out_msg_size];
> _agent_out_msg->protocol = VD_AGENT_PROTOCOL;
> _agent_out_msg->type = VD_AGENT_CLIPBOARD;
> _agent_out_msg->opaque = 0;
> - _agent_out_msg->size = sizeof(VDAgentClipboard) + clip_size;
> + _agent_out_msg->size = sizeof(VDAgentClipboard) + size;
> VDAgentClipboard* clipboard = (VDAgentClipboard*)_agent_out_msg->data;
> - clipboard->type = get_agent_clipboard_type(clip_type);
> - if (!Platform::get_clipboard_data(clip_type, clipboard->data, clip_size)) {
> - delete[] (uint8_t *)_agent_out_msg;
> - _agent_out_msg = NULL;
> - _agent_out_msg_size = 0;
> - return;
> - }
> -
> + clipboard->type = get_agent_clipboard_type(type);
> + memcpy(clipboard->data, data, size);
> if (_agent_tokens) {
> do_send_agent_clipboard();
> }
> }
>
> -void RedClient::on_clipboard_change()
> -{
> - AutoRef<ClipboardEvent> event(new ClipboardEvent());
> - get_process_loop().push_event(*event);
> -}
> -
> void RedClient::set_mouse_mode(uint32_t supported_modes, uint32_t current_mode)
> {
> if (current_mode != _mouse_mode) {
### start "these functions"
> @@ -886,13 +909,62 @@ void RedClient::set_mouse_mode(uint32_t supported_modes, uint32_t current_mode)
>
> void RedClient::on_agent_clipboard(VDAgentClipboard* clipboard, uint32_t size)
> {
> - switch (clipboard->type) {
> - case VD_AGENT_CLIPBOARD_UTF8_TEXT:
> - Platform::set_clipboard_data(Platform::CLIPBOARD_UTF8_TEXT, clipboard->data, size);
> - break;
> - default:
> - THROW("unexpected vdagent clipboard data type");
> + uint32_t type = get_platform_clipboard_type(clipboard->type);
> +
> + if (!type) {
> + LOG_INFO("Unsupported clipboard type %u", clipboard->type);
> + return;
> }
> + Platform::set_clipboard_data(type, clipboard->data, size);
> +}
> +
> +void RedClient::on_agent_clipboard_grab(VDAgentClipboardGrab* clipboard_grab)
> +{
> + uint32_t type = get_platform_clipboard_type(clipboard_grab->type);
> +
> + if (!type) {
> + LOG_INFO("Unsupported clipboard type %u", clipboard_grab->type);
> + return;
> + }
> + Platform::set_clipboard_owner(type);
> +}
> +
> +void RedClient::on_agent_clipboard_request(VDAgentClipboardRequest* clipboard_request)
> +{
> + uint32_t type = get_platform_clipboard_type(clipboard_request->type);
> +
> + if (!type) {
> + LOG_INFO("Unsupported clipboard type %u", clipboard_request->type);
> + return;
> + }
> + if (!Platform::request_clipboard_notification(type)) {
> + send_agent_clipboard_message(VD_AGENT_CLIPBOARD_RELEASE);
> + }
> +}
> +
> +void RedClient::on_agent_clipboard_release()
> +{
> + Platform::release_clipboard();
> +}
> +
It seems to me that "these" functions are a but superfluous, making it harder
to follow the call chain (esp once get_platform_clipboard_type is no more).
I think it would be better to just directly call the relevant
Platform::*clipboard* functions directly from the dispatch_agent_message()
switch case, this removes a nice number of lines and makes the call chain
easier to follow.
> +uint32_t RedClient::get_platform_clipboard_type(uint32_t agent_type)
> +{
> + for (ClipboardType* iter = clipboard_types; iter->agent_type&& iter->platform_type; iter++) {
> + if (iter->agent_type == agent_type) {
> + return iter->platform_type;
> + }
> + }
> + return 0;
> +}
> +
> +uint32_t RedClient::get_agent_clipboard_type(uint32_t platform_type)
> +{
> + for (ClipboardType* iter = clipboard_types; iter->agent_type&& iter->platform_type; iter++) {
> + if (iter->platform_type == platform_type) {
> + return iter->agent_type;
> + }
> + }
> + return 0;
> }
>
As discussed before I believe these are unnecessary.
> void RedClient::handle_init(RedPeer::InMessage* message)
> @@ -1055,25 +1127,7 @@ void RedClient::handle_agent_data(RedPeer::InMessage* message)
> }
> if (_agent_msg_pos == sizeof(VDAgentMessage) + _agent_msg->size) {
> DBG(0, "agent msg end");
> - switch (_agent_msg->type) {
> - case VD_AGENT_ANNOUNCE_CAPABILITIES: {
> - on_agent_announce_capabilities(
> - (VDAgentAnnounceCapabilities*)_agent_msg_data,
> - _agent_msg->size);
> - break;
> - }
> - case VD_AGENT_REPLY: {
> - on_agent_reply((VDAgentReply*)_agent_msg_data);
> - break;
> - }
> - case VD_AGENT_CLIPBOARD: {
> - on_agent_clipboard((VDAgentClipboard*)_agent_msg_data,
> - _agent_msg->size - sizeof(VDAgentClipboard));
> - break;
> - }
> - default:
> - DBG(0, "Unsupported message type %u size %u", _agent_msg->type, _agent_msg->size);
> - }
> + dispatch_agent_message(_agent_msg, _agent_msg_data);
> delete[] _agent_msg_data;
> _agent_msg_data = NULL;
> _agent_msg_pos = 0;
> @@ -1081,6 +1135,35 @@ void RedClient::handle_agent_data(RedPeer::InMessage* message)
> }
> }
>
> +void RedClient::dispatch_agent_message(VDAgentMessage* msg, void* data)
> +{
> + switch (msg->type) {
> + case VD_AGENT_ANNOUNCE_CAPABILITIES: {
> + on_agent_announce_capabilities((VDAgentAnnounceCapabilities*)data, msg->size);
> + break;
> + }
> + case VD_AGENT_REPLY: {
> + on_agent_reply((VDAgentReply*)data);
> + break;
> + }
> + case VD_AGENT_CLIPBOARD: {
> + on_agent_clipboard((VDAgentClipboard*)data, msg->size - sizeof(VDAgentClipboard));
> + break;
> + }
> + case VD_AGENT_CLIPBOARD_GRAB:
> + on_agent_clipboard_grab((VDAgentClipboardGrab*)data);
> + break;
> + case VD_AGENT_CLIPBOARD_REQUEST:
> + on_agent_clipboard_request((VDAgentClipboardRequest*)data);
> + break;
> + case VD_AGENT_CLIPBOARD_RELEASE:
> + on_agent_clipboard_release();
> + break;
> + default:
> + DBG(0, "Unsupported message type %u size %u", msg->type, msg->size);
> + }
> +}
> +
> void RedClient::handle_agent_tokens(RedPeer::InMessage* message)
> {
> SpiceMsgMainAgentTokens *token = (SpiceMsgMainAgentTokens *)message->data();
> diff --git a/client/red_client.h b/client/red_client.h
> index ba8b4ee..ea9c735 100644
> --- a/client/red_client.h
> +++ b/client/red_client.h
> @@ -144,16 +144,31 @@ public:
> uint32_t _color_depth;
> };
>
> -class ClipboardEvent : public Event {
> +class ClipboardGrabEvent : public Event {
> public:
> + ClipboardGrabEvent(uint32_t type) : _type (type) {}
> virtual void response(AbstractProcessLoop& events_loop);
> +
> +private:
> + uint32_t _type;
> };
>
> -class RedClient: public RedChannel {
> +class ClipboardRequestEvent : public Event {
> +public:
> + ClipboardRequestEvent(uint32_t type) : _type (type) {}
> + virtual void response(AbstractProcessLoop& events_loop);
> +
> +private:
> + uint32_t _type;
> +};
> +
> +class RedClient: public RedChannel,
> + public Platform::ClipboardListener {
> public:
> friend class RedChannel;
> friend class Migrate;
> - friend class ClipboardEvent;
> + friend class ClipboardGrabEvent;
> + friend class ClipboardRequestEvent;
>
> RedClient(Application& application);
> ~RedClient();
> @@ -192,7 +207,10 @@ public:
> PixmapCache& get_pixmap_cache() {return _pixmap_cache;}
> uint64_t get_pixmap_cache_size() { return _pixmap_cache_size;}
> void on_display_mode_change();
> - void on_clipboard_change();
> + void on_clipboard_change(uint32_t type);
> + void on_clipboard_render(uint32_t type);
> + void on_clipboard_notify(uint32_t type, uint8_t* data, int32_t size);
> +
> void for_each_channel(ForEachChannelFunc& func);
> void on_mouse_capture_trigger(RedScreen& screen);
>
> @@ -228,13 +246,19 @@ private:
> void handle_agent_data(RedPeer::InMessage* message);
> void handle_agent_tokens(RedPeer::InMessage* message);
> void handle_migrate_switch_host(RedPeer::InMessage* message);
> + void dispatch_agent_message(VDAgentMessage* msg, void* data);
>
> void on_agent_reply(VDAgentReply* reply);
> void on_agent_announce_capabilities(VDAgentAnnounceCapabilities* caps,
> uint32_t msg_size);
> void on_agent_clipboard(VDAgentClipboard* clipboard, uint32_t size);
> - void send_agent_clipboard();
> + void on_agent_clipboard_grab(VDAgentClipboardGrab* clipboard_grab);
> + void on_agent_clipboard_request(VDAgentClipboardRequest* clipboard_request);
> + void on_agent_clipboard_release();
> void do_send_agent_clipboard();
> + void send_agent_clipboard_message(uint32_t message_type, uint32_t size = 0, void* data = NULL);
> + uint32_t get_platform_clipboard_type(uint32_t agent_type);
> + uint32_t get_agent_clipboard_type(uint32_t platform_type);
>
> ChannelFactory* find_factory(uint32_t type);
> void create_channel(uint32_t type, uint32_t id);
> diff --git a/client/windows/platform.cpp b/client/windows/platform.cpp
> index ae49d02..096cd66 100644
> --- a/client/windows/platform.cpp
> +++ b/client/windows/platform.cpp
> @@ -45,9 +45,35 @@ public:
>
> static DefaultEventListener default_event_listener;
> static Platform::EventListener* event_listener =&default_event_listener;
> -static HWND paltform_win;
> +static HWND paltform_win = NULL;
Typo s/paltform_win/platform_win/g
> static ProcessLoop* main_loop = NULL;
>
> +class DefaultClipboardListener: public Platform::ClipboardListener {
> +public:
> + virtual void on_clipboard_change(uint32_t type) {}
> + virtual void on_clipboard_render(uint32_t type) {}
> + virtual void on_clipboard_notify(uint32_t type, uint8_t* data, int32_t size) {}
> +};
> +
> +static DefaultClipboardListener default_clipboard_listener;
> +static Platform::ClipboardListener* clipboard_listener =&default_clipboard_listener;
> +
> +// The next window in the clipboard viewer chain, which is refered in all clipboard events.
> +static HWND next_clipboard_viewer_win = NULL;
> +static HANDLE clipboard_event = NULL;
> +
> +// clipboard_changer says whether the client was the last one to change cliboard, for loop
> +// prevention. It's initialized to true so we ignore the first clipboard change event which
> +// happens right when we call SetClipboardViewer().
> +static bool clipboard_changer = true;
> +
> +static const int CLIPBOARD_TIMEOUT_MS = 10000;
> +
> +Platform::ClipboardFormat Platform::clipboard_formats[] = {
> + {CF_UNICODETEXT, Platform::CLIPBOARD_UTF8_TEXT},
> + {CF_DIB, Platform::CLIPBOARD_BITMAP},
> + {0, 0}};
> +
> static const unsigned long MODAL_LOOP_TIMER_ID = 1;
> static const int MODAL_LOOP_DEFAULT_TIMEOUT = 100;
> static bool modal_loop_active = false;
> @@ -59,6 +85,19 @@ void Platform::send_quit_request()
> main_loop->quit(0);
> }
>
> +static uint32_t get_available_clipboard_type()
> +{
> + uint32_t type = 0;
> +
> + for (Platform::ClipboardFormat* iter = Platform::clipboard_formats;
> + iter->format&& !type; iter++) {
> + if (IsClipboardFormatAvailable(iter->format)) {
> + type = iter->type;
> + }
> + }
> + return type;
> +}
> +
> static LRESULT CALLBACK PlatformWinProc(HWND hWnd, UINT message, WPARAM wParam, LPARAM lParam)
> {
> switch (message) {
> @@ -82,6 +121,44 @@ static LRESULT CALLBACK PlatformWinProc(HWND hWnd, UINT message, WPARAM wParam,
> case WM_DISPLAYCHANGE:
> event_listener->on_monitors_change();
> break;
> + case WM_CHANGECBCHAIN:
> + if (next_clipboard_viewer_win == (HWND)wParam) {
> + next_clipboard_viewer_win = (HWND)lParam;
> + } else if (next_clipboard_viewer_win) {
> + SendMessage(next_clipboard_viewer_win, message, wParam, lParam);
> + }
> + break;
> + case WM_DRAWCLIPBOARD:
> + if (!clipboard_changer) {
> + uint32_t type = get_available_clipboard_type();
> + if (type) {
> + clipboard_listener->on_clipboard_change(type);
> + } else {
> + LOG_INFO("Unsupported clipboard format");
> + }
> + } else {
> + clipboard_changer = false;
> + }
> + if (next_clipboard_viewer_win) {
> + SendMessage(next_clipboard_viewer_win, message, wParam, lParam);
> + }
> + break;
> + case WM_RENDERFORMAT: {
> + // In delayed rendering, Windows requires us to SetClipboardData before we return from
> + // handling WM_RENDERFORMAT. Therefore, we try our best by sending CLIPBOARD_REQUEST to the
> + // agent, while waiting alertably for a while (hoping for good) for receiving CLIPBOARD data
> + // or CLIPBOARD_RELEASE from the agent, which both will signal clipboard_event.
> + uint32_t type = Platform::get_clipboard_type(wParam);
> + if (!type) {
> + LOG_INFO("Unsupported clipboard format %u", wParam);
> + break;
> + }
> + clipboard_listener->on_clipboard_render(type);
> + DWORD start_tick = GetTickCount();
> + while (WaitForSingleObjectEx(clipboard_event, 1000, TRUE) != WAIT_OBJECT_0&&
> + GetTickCount()< start_tick + CLIPBOARD_TIMEOUT_MS);
> + break;
> + }
> default:
> return DefWindowProc(hWnd, message, wParam, lParam);
> }
> @@ -92,7 +169,6 @@ static void create_message_wind()
> {
> WNDCLASSEX wclass;
> ATOM class_atom;
> - HWND window;
>
> const LPCWSTR class_name = L"spicec_platform_wclass";
>
> @@ -113,11 +189,16 @@ static void create_message_wind()
> THROW("register class failed");
> }
>
> - if (!(window = CreateWindow(class_name, L"", 0, 0, 0, 0, 0, NULL, NULL, instance, NULL))) {
> + if (!(paltform_win = CreateWindow(class_name, L"", 0, 0, 0, 0, 0, NULL, NULL, instance, NULL))) {
> THROW("create message window failed");
> }
>
> - paltform_win = window;
> + if (!(next_clipboard_viewer_win = SetClipboardViewer(paltform_win))&& GetLastError()) {
> + THROW("set clipboard viewer failed");
> + }
> + if (!(clipboard_event = CreateEvent(NULL, FALSE, FALSE, NULL))) {
> + THROW("create clipboard event failed");
> + }
> }
>
> NamedPipe::ListenerRef NamedPipe::create(const char *name, ListenerInterface& listener_interface)
> @@ -460,9 +541,16 @@ void Platform::path_append(std::string& path, const std::string& partial_path)
> path += partial_path;
> }
>
> +static void cleanup()
> +{
> + ChangeClipboardChain(paltform_win, next_clipboard_viewer_win);
> + CloseHandle(clipboard_event);
> +}
> +
> void Platform::init()
> {
> create_message_wind();
> + atexit(cleanup);
> }
>
> void Platform::set_process_loop(ProcessLoop& main_process_loop)
> @@ -749,62 +837,101 @@ void WinPlatform::exit_modal_loop()
> modal_loop_active = false;
> }
>
> -void Platform::set_clipboard_listener(ClipboardListener* listener)
> +bool Platform::set_clipboard_owner(uint32_t type)
> {
> - //FIXME: call only on change, use statics
> - listener->on_clipboard_change();
> + uint32_t format = get_clipboard_format(type);
> +
> + if (!format) {
> + LOG_INFO("Unsupported clipboard type %u", type);
> + return false;
> + }
> + if (!OpenClipboard(paltform_win)) {
> + return false;
> + }
> + clipboard_changer = true;
> + EmptyClipboard();
> + SetClipboardData(format, NULL);
> + CloseClipboard();
> + return true;
> }
>
> -UINT get_format(uint32_t type)
> +void Platform::set_clipboard_listener(ClipboardListener* listener)
> {
> - switch (type) {
> - case Platform::CLIPBOARD_UTF8_TEXT:
> - return CF_UNICODETEXT;
> - default:
> - return 0;
> - }
> + clipboard_listener = listener ? listener :&default_clipboard_listener;
> }
>
> bool Platform::set_clipboard_data(uint32_t type, const uint8_t* data, int32_t size)
> {
> - UINT format = get_format(type);
> HGLOBAL clip_data;
> LPVOID clip_buf;
> int clip_size;
> - bool ret;
> -
> - //LOG_INFO("type %u size %d %s", type, size, data);
> - if (!format || !OpenClipboard(paltform_win)) {
> + int clip_len;
> + UINT format;
> + bool ret = false;
> +
> + // Get the required clipboard size
> + switch (format = get_clipboard_format(type)) {
No need to do a switch case on the format here, you might
as well do it directly on the type given that there is a
1:1 mapping.
> + case CF_UNICODETEXT:
> + // Received utf8 string is not null-terminated
> + if (!(clip_len = MultiByteToWideChar(CP_UTF8, 0, (LPCSTR)data, size, NULL, 0))) {
> + return false;
> + }
> + clip_len++;
> + clip_size = clip_len * sizeof(WCHAR);
> + break;
> + case CF_DIB:
> + clip_size = size;
> + break;
> + default:
> + LOG_INFO("Unsupported clipboard type %u", type);
> return false;
> }
> - clip_size = MultiByteToWideChar(CP_UTF8, 0, (LPCSTR)data, size, NULL, 0);
> - if (!clip_size || !(clip_data = GlobalAlloc(GMEM_DDESHARE, clip_size * sizeof(WCHAR)))) {
> - CloseClipboard();
> +
> + // Allocate and lock clipboard memory
> + if (!(clip_data = GlobalAlloc(GMEM_DDESHARE, clip_size))) {
> return false;
> }
> if (!(clip_buf = GlobalLock(clip_data))) {
> GlobalFree(clip_data);
> - CloseClipboard();
> return false;
> }
> - ret = !!MultiByteToWideChar(CP_UTF8, 0, (LPCSTR)data, size, (LPWSTR)clip_buf, clip_size);
> +
> + // Translate data and set clipboard content
> + switch (format) {
> + case CF_UNICODETEXT:
> + ret = !!MultiByteToWideChar(CP_UTF8, 0, (LPCSTR)data, size, (LPWSTR)clip_buf, clip_len);
> + ((LPWSTR)clip_buf)[clip_len - 1] = L'\0';
> + break;
> + case CF_DIB:
> + memcpy(clip_buf, data, size);
> + ret = true;
> + break;
> + }
> GlobalUnlock(clip_data);
> - if (ret) {
> - EmptyClipboard();
> - ret = !!SetClipboardData(format, clip_data);
> + if (!ret) {
> + return false;
> + }
> + if (SetClipboardData(format, clip_data)) {
> + SetEvent(clipboard_event);
> + return true;
> + }
> + // We retry clipboard open-empty-set-close only when there is a timeout in WM_RENDERFORMAT
> + if (!OpenClipboard(paltform_win)) {
> + return false;
> }
> + EmptyClipboard();
> + ret = !!SetClipboardData(format, clip_data);
> CloseClipboard();
> return ret;
> }
>
> -bool Platform::get_clipboard_data(uint32_t type, uint8_t* data, int32_t size)
> +bool Platform::request_clipboard_notification(uint32_t type)
> {
> - UINT format = get_format(type);
> + UINT format = get_clipboard_format(type);
> HANDLE clip_data;
> LPVOID clip_buf;
> - bool ret;
> + bool ret = false;
>
> - LOG_INFO("type %u size %d", type, size);
> if (!format || !IsClipboardFormatAvailable(format) || !OpenClipboard(paltform_win)) {
> return false;
> }
> @@ -812,32 +939,45 @@ bool Platform::get_clipboard_data(uint32_t type, uint8_t* data, int32_t size)
> CloseClipboard();
> return false;
> }
> - ret = !!WideCharToMultiByte(CP_UTF8, 0, (LPCWSTR)clip_buf, -1, (LPSTR)data, size, NULL, NULL);
> - GlobalUnlock(clip_data);
> - CloseClipboard();
> - return ret;
> -}
> -
> -int32_t Platform::get_clipboard_data_size(uint32_t type)
> -{
> - UINT format = get_format(type);
> - HANDLE clip_data;
> - LPVOID clip_buf;
> - int clip_size;
>
> - if (!format || !IsClipboardFormatAvailable(format) || !OpenClipboard(paltform_win)) {
> - return 0;
> + switch (format) {
> + case CF_UNICODETEXT: {
> + size_t len = wcslen((wchar_t*)clip_buf);
> + int utf8_size = WideCharToMultiByte(CP_UTF8, 0, (LPCWSTR)clip_buf, len, NULL, 0, NULL, NULL);
> + if (!utf8_size) {
> + break;
> + }
> + uint8_t* utf8_data = new uint8_t[utf8_size];
> + if (WideCharToMultiByte(CP_UTF8, 0, (LPCWSTR)clip_buf, len, (LPSTR)utf8_data, utf8_size,
> + NULL, NULL)) {
> + clipboard_listener->on_clipboard_notify(type, utf8_data, utf8_size);
> + ret = true;
> + }
> + delete[] (uint8_t *)utf8_data;
> + break;
> }
> - if (!(clip_data = GetClipboardData(format)) || !(clip_buf = GlobalLock(clip_data))) {
> - CloseClipboard();
> - return 0;
> + case CF_DIB: {
> + size_t clip_size = GlobalSize(clip_data);
> + if (!clip_size) {
> + break;
> + }
> + clipboard_listener->on_clipboard_notify(type, (uint8_t*)clip_buf, clip_size);
> + ret = true;
> + break;
> + }
> + default:
> + LOG_INFO("Unsupported clipboard type %u", type);
> }
> - clip_size = WideCharToMultiByte(CP_UTF8, 0, (LPCWSTR)clip_buf, -1, NULL, 0, NULL, NULL);
> +
> GlobalUnlock(clip_data);
> CloseClipboard();
> - return clip_size;
> + return ret;
> }
>
> +void Platform::release_clipboard()
> +{
> + SetEvent(clipboard_event);
> +}
>
> static bool has_console = false;
>
> diff --git a/client/x11/Makefile.am b/client/x11/Makefile.am
> index 101f6dd..2a83473 100644
> --- a/client/x11/Makefile.am
> +++ b/client/x11/Makefile.am
> @@ -22,6 +22,7 @@ INCLUDES = \
> $(CELT051_CFLAGS) \
> $(SSL_CFLAGS) \
> $(XRANDR_CFLAGS) \
> + $(XFIXES_LIBS) \
> $(MISC_X_CFLAGS) \
> $(CEGUI_CFLAGS) \
> $(WARN_CFLAGS) \
> diff --git a/client/x11/platform.cpp b/client/x11/platform.cpp
> index cc1502b..625af2a 100644
> --- a/client/x11/platform.cpp
> +++ b/client/x11/platform.cpp
> @@ -28,6 +28,7 @@
> #include<X11/extensions/XKB.h>
> #include<X11/extensions/Xrender.h>
> #include<X11/extensions/XShm.h>
> +#include<X11/extensions/Xfixes.h>
> #include<unistd.h>
> #include<sys/socket.h>
> #include<sys/epoll.h>
> @@ -64,7 +65,7 @@
> //#define X_DEBUG_SYNC(display) XSync(display, False)
> #define X_DEBUG_SYNC(display)
> #ifdef HAVE_XRANDR12
> -//#define USE_XRANDR_1_2
> +#define USE_XRANDR_1_2
> #endif
>
> static Display* x_display = NULL;
> @@ -94,24 +95,33 @@ static int xrandr_minor = 0;
>
> static bool using_xrender_0_5 = false;
>
> +static bool using_xfixes_1_0 = false;
> +
> +static int xfixes_event_base;
> +static int xfixes_error_base;
> +
> static unsigned int caps_lock_mask = 0;
> static unsigned int num_lock_mask = 0;
>
> //FIXME: nicify
> static uint8_t* clipboard_data = NULL;
> +static int32_t clipboard_data_type = 0;
> static int32_t clipboard_data_size = 0;
> static int32_t clipboard_data_space = 0;
> +static int32_t clipboard_request_type = 0;
> +static bool clipboard_changer = false;
> +static XEvent clipboard_event;
> static Mutex clipboard_lock;
> static Atom clipboard_prop;
> static Atom incr_atom;
> static Atom utf8_atom;
> -//static Atom clipboard_type_utf8;
> -#ifdef USE_XRANDR_1_2
> -static bool clipboard_inited = false;
> -#endif
> static Bool handle_x_error = false;
> static int x_error_code;
>
> +Platform::ClipboardFormat Platform::clipboard_formats[] = {
> + {0, 0},
> + {0, 0}};
> +
> class DefaultEventListener: public Platform::EventListener {
> public:
> virtual void on_app_activated() {}
> @@ -132,7 +142,9 @@ static Platform::DisplayModeListener* display_mode_listener =&default_display_m
>
> class DefaultClipboardListener: public Platform::ClipboardListener {
> public:
> - void on_clipboard_change() {}
> + void on_clipboard_change(uint32_t type) {}
> + void on_clipboard_render(uint32_t type) {}
> + void on_clipboard_notify(uint32_t type, uint8_t* data, int32_t size) {}
> };
>
> static DefaultClipboardListener default_clipboard_listener;
> @@ -763,6 +775,10 @@ static void intern_clipboard_atoms()
> clipboard_prop = XInternAtom(x_display, "CLIPBOARD", False);
> incr_atom = XInternAtom(x_display, "INCR", False);
> utf8_atom = XInternAtom(x_display, "UTF8_STRING", False);
> +
> + Platform::clipboard_formats[0].format = utf8_atom;
> + Platform::clipboard_formats[0].type = Platform::CLIPBOARD_UTF8_TEXT;
> +
> interned = true;
> }
>
> @@ -778,13 +794,19 @@ DynamicScreen::DynamicScreen(Display* display, int screen, int& next_mon_id)
> platform_win = XCreateSimpleWindow(display, RootWindow(display, screen), 0, 0, 1, 1, 0, 0, 0);
> XSelectInput(display, platform_win, StructureNotifyMask);
> XRRSelectInput(display, platform_win, RRScreenChangeNotifyMask);
> + if (using_xfixes_1_0) {
> + XFixesSelectSelectionInput(display, platform_win, XA_PRIMARY,
> + XFixesSetSelectionOwnerNotifyMask |
> + XFixesSelectionWindowDestroyNotifyMask |
> + XFixesSelectionClientCloseNotifyMask);
> + }
>
> Monitor::self_monitors_change++;
> process_monitor_configure_events(platform_win);
> Monitor::self_monitors_change--;
>
> - XPlatform::set_win_proc(platform_win, root_win_proc);
> intern_clipboard_atoms();
> + XPlatform::set_win_proc(platform_win, root_win_proc);
> X_DEBUG_SYNC(display);
> }
>
> @@ -873,7 +895,7 @@ bool DynamicScreen::set_screen_size(int size_index)
> Monitor::self_monitors_change++;
> /*what status*/
> XRRSetScreenConfig(get_display(), config, root_window, size_index, rotation, CurrentTime);
> - process_monitor_configure_events(root_window);
> + process_monitor_configure_events(platform_win);
> Monitor::self_monitors_change--;
> XRRFreeScreenConfigInfo(config);
> X_DEBUG_SYNC(get_display());
> @@ -1045,26 +1067,24 @@ MultyMonScreen::MultyMonScreen(Display* display, int screen, int& next_mon_id)
> throw;
> }
>
> - XSelectInput(display, root_window, StructureNotifyMask);
> + platform_win = XCreateSimpleWindow(display, RootWindow(display, screen), 0, 0, 1, 1, 0, 0, 0);
> + XSelectInput(display, platform_win, StructureNotifyMask);
Not directly related to this patch, but why the creation and use
of a platform win rather then directly use the root window?
> X_DEBUG_SYNC(get_display());
> - XRRSelectInput(display, root_window, RRScreenChangeNotifyMask);
> + XRRSelectInput(display, platform_win, RRScreenChangeNotifyMask);
> X_DEBUG_SYNC(get_display());
> - intern_clipboard_atoms();
> - XPlatform::set_win_proc(root_window, root_win_proc);
> + if (using_xfixes_1_0) {
> + XFixesSelectSelectionInput(display, platform_win, XA_PRIMARY,
> + XFixesSetSelectionOwnerNotifyMask |
> + XFixesSelectionWindowDestroyNotifyMask |
> + XFixesSelectionClientCloseNotifyMask);
> + }
>
> XMonitor::inc_change_ref();
> - process_monitor_configure_events(root_window);
> + process_monitor_configure_events(platform_win);
> XMonitor::dec_change_ref();
>
> - X_DEBUG_SYNC(get_display());
> - //
> - //platform_win = XCreateSimpleWindow(display, RootWindow(display, screen), 0, 0, 1, 1, 0, 0, 0);
> - //XSelectInput(display, platform_win, StructureNotifyMask);
> - //XRRSelectInput(display, platform_win, RRScreenChangeNotifyMask);
> - //XPlatform::set_win_proc(platform_win, root_win_proc);
> - //clipboard_prop = XInternAtom(x_display, "CLIPBOARD", False);
> - //
> - clipboard_inited = true;
> + intern_clipboard_atoms();
> + XPlatform::set_win_proc(platform_win, root_win_proc);
> X_DEBUG_SYNC(get_display());
> }
>
> @@ -1172,7 +1192,7 @@ void MultyMonScreen::restore()
> (*iter)->revert();
> }
> enable();
> - process_monitor_configure_events(root_window);
> + process_monitor_configure_events(platform_win);
> XMonitor::dec_change_ref();
> X_DEBUG_SYNC(get_display());
> }
> @@ -1734,8 +1754,7 @@ bool MultyMonScreen::set_monitor_mode(XMonitor& monitor, const XRRModeInfo& mode
> monitor.set_mode(mode_info);
> set_size(screen_width, screen_height);
> enable();
> - Window root_window = RootWindow(get_display(), get_screen());
> - process_monitor_configure_events(root_window);
> + process_monitor_configure_events(platform_win);
> XMonitor::dec_change_ref();
> X_DEBUG_SYNC(get_display());
> return true;
> @@ -2193,7 +2212,9 @@ static void ensure_clipboard_data_space(uint32_t size)
>
> static void realloc_clipboard_data_space(uint32_t size)
> {
> - if (size<= clipboard_data_space) return;
> + if (size<= clipboard_data_space) {
> + return;
> + }
> uint32_t old_alloc = clipboard_data_space;
> clipboard_data_space = size;
> uint8_t *newbuf = new uint8_t[clipboard_data_space];
> @@ -2211,49 +2232,58 @@ static void update_clipboard(unsigned long size, uint8_t* data)
> clipboard_data_size = size;
> }
>
> -
> -/* NOTE: Function taken from xsel, original name get_append_property
> - *
> - * Get a window clipboard property and append its data to the clipboard_data
> - *
> - * Returns true if more data is available for receipt.
> - *
> - * Returns false if no data is availab, or on error.
> - */
> -static bool
> -get_append_clipboard_data (XSelectionEvent* xsel)
> -{
> - Atom target;
> - int format;
> - unsigned long bytesafter, length;
> - unsigned char * value;
> -
> - XGetWindowProperty (x_display, xsel->requestor, clipboard_prop,
> - 0L, 1000000, True, (Atom)AnyPropertyType,
> -&target,&format,&length,&bytesafter,&value);
> -
> - if (target != utf8_atom) {
> - LOG_INFO ("%s: target %d not UTF8", __func__, target);
> - // realloc clipboard_data to 0?
> - return false;
> - } else if (length == 0) {
> - /* A length of 0 indicates the end of the transfer */
> - LOG_INFO ("Got zero length property; end of INCR transfer");
> - return false;
> - } else if (format == 8) {
> +// Function based on xsel get_append_property()
> +// Get a window clipboard property and append its data to the clipboard_data
> +// Returns true if more data is available for receipt. false if no data is available, or on error.
> +static bool get_append_clipboard_data(XSelectionEvent* xsel)
> +{
> + Atom target;
> + int format;
> + unsigned long bytesafter, length;
> + unsigned char* value;
> +
> + XGetWindowProperty(x_display, xsel->requestor, clipboard_prop, 0L, 1000000, True,
> + xsel->target,&target,&format,&length,&bytesafter,&value);
> + if (target != xsel->target) {
> + LOG_INFO("target %d != %u requested", target, xsel->target);
> + clipboard_data_size = 0;
> + return false;
> + } else if (length == 0) {
> + // A length of 0 indicates the end of the transfer
> + LOG_INFO("Got zero length property; end of INCR transfer");
> + return false;
> + } else if (format != 8) {
> + LOG_INFO("Retrieved non-8-bit data");
> + clipboard_data_size = 0;
> + return false;
> + }
> if (clipboard_data_size + length> clipboard_data_space) {
> - realloc_clipboard_data_space(clipboard_data_size + length);
> + realloc_clipboard_data_space(clipboard_data_size + length);
> }
> - strncpy ((char*)clipboard_data + clipboard_data_size, (char*)value, length);
> + strncpy((char*)clipboard_data + clipboard_data_size, (char*)value, length);
Why strncpy and not memcpy? given that we've a length memcpy seems better
suited for this.
> clipboard_data_size += length;
> - LOG_INFO ("Appended %d bytes to buffer\n", length);
> - } else {
> - LOG_WARN ("Retrieved non-8-bit data\n");
> - }
> -
> - return true;
> + LOG_INFO("Appended %d bytes to buffer", length);
> + return true;
> }
>
> +// FIXME: use INCR for large data transfers
> +static void send_selection_notify(Atom type)
> +{
> + Window requestor_win = clipboard_event.xselectionrequest.requestor;
> + Atom prop = clipboard_event.xselectionrequest.property;
> + XChangeProperty(x_display, requestor_win, prop, type, 8, PropModeReplace,
> + (unsigned char *)clipboard_data, clipboard_data_size);
> + XEvent res;
> + res.xselection.property = prop;
> + res.xselection.type = SelectionNotify;
> + res.xselection.display = clipboard_event.xselectionrequest.display;
> + res.xselection.requestor = requestor_win;
> + res.xselection.selection = clipboard_event.xselectionrequest.selection;
> + res.xselection.target = clipboard_event.xselectionrequest.target;
> + res.xselection.time = clipboard_event.xselectionrequest.time;
> + XSendEvent(x_display, requestor_win, 0, 0,&res);
> + XFlush(x_display);
> +}
>
> static void root_win_proc(XEvent& event)
> {
> @@ -2281,28 +2311,38 @@ static void root_win_proc(XEvent& event)
> event_listener->on_monitors_change();
> return;
> }
> -
> + if (event.type == XFixesSelectionNotify + xfixes_event_base) {
> + XFixesSelectionNotifyEvent* selection_event = (XFixesSelectionNotifyEvent *)&event;
> + if (selection_event->subtype != XFixesSetSelectionOwnerNotify) {
> + // FIXME: for some reason the XFixesSelectionWindowDestroyNotify/ClientCloseNotify
> + // events which can help for sending CLIPBOARD_RELEASE to the agent are not received
This FIXME seems to me to be caused by you only checking for XFixesSelectionNotify in the
outer if of this nested if.
> + LOG_INFO("Unsupported selection event %u", selection_event->subtype);
> + return;
> + }
> + LOG_INFO("XFixesSetSelectionOwnerNotify %u", clipboard_changer);
> + if (clipboard_changer) {
> + clipboard_changer = false;
> + return;
> + }
> + // FIXME: use actual type
> + clipboard_listener->on_clipboard_change(Platform::CLIPBOARD_UTF8_TEXT);
> + return;
> + }
> switch (event.type) {
> case SelectionRequest: {
> - //FIXME: support multi-chunk
> Lock lock(clipboard_lock);
> - if (clipboard_data_size == 0) {
> - return;
> + XSelectionRequestEvent* selection_request = (XSelectionRequestEvent*)&event;
> + uint32_t type = Platform::get_clipboard_type(selection_request->target);
> + if (!type) {
> + LOG_INFO("Unsupported selection type %u", selection_request->target);
> + break;
> + }
> + if (clipboard_data_size> 0) {
> + send_selection_notify(selection_request->target);
> + break;
> }
This seems wrong, if an app requests a selection it should get the current data from
the agent, not whatever happens to still be in the buffer from when a previous app
requested it but happened to close / crash before consuming the selection. Also the
target for the old data could be different (once we support multiple targets).
> - Window requestor_win = event.xselectionrequest.requestor;
> - Atom prop = event.xselectionrequest.property;
> - XChangeProperty(x_display, requestor_win, prop, utf8_atom, 8, PropModeReplace,
> - (unsigned char *)clipboard_data, clipboard_data_size);
> - XEvent res;
> - res.xselection.property = prop;
> - res.xselection.type = SelectionNotify;
> - res.xselection.display = event.xselectionrequest.display;
> - res.xselection.requestor = requestor_win;
> - res.xselection.selection = event.xselectionrequest.selection;
> - res.xselection.target = event.xselectionrequest.target;
> - res.xselection.time = event.xselectionrequest.time;
> - XSendEvent(x_display, requestor_win, 0, 0,&res);
> - XFlush(x_display);
> + clipboard_event = event;
> + clipboard_listener->on_clipboard_render(type);
> break;
> }
> case SelectionClear: {
> @@ -2317,17 +2357,26 @@ static void root_win_proc(XEvent& event)
> unsigned long size;
> unsigned long dummy;
> unsigned char *data;
> +
> XGetWindowProperty(x_display, platform_win, clipboard_prop, 0, 0, False,
> - AnyPropertyType,&type,&format,&len,&size,&data);
> + event.xselection.target,&type,&format,&len,&size,&data);
> if (size == 0) {
> + LOG_INFO("XGetWindowProperty(size) failed");
> + break;
> + }
> + if (type != event.xselection.target) {
> + LOG_INFO("type %d != %u requested", type, event.xselection.target);
> break;
> }
> if (XGetWindowProperty(x_display, platform_win, clipboard_prop, 0, size,
> - False, AnyPropertyType,&type,&format,&len,&dummy,&data) != Success) {
> - LOG_INFO("XGetWindowProperty failed");
> + False, event.xselection.target,&type,&format,&len,&dummy,&data) != Success) {
> + LOG_INFO("XGetWindowProperty(data) failed");
> + break;
> + }
> + if (type != event.xselection.target) {
> + LOG_INFO("type %d != %u requested", type, event.xselection.target);
> break;
> }
> - LOG_INFO("data: %s len: %u", data, len);
> {
> Lock lock(clipboard_lock);
> clipboard_data_size = 0;
> @@ -2336,7 +2385,7 @@ static void root_win_proc(XEvent& event)
> Window requestor_win = event.xselection.requestor;
> Atom prop = event.xselection.property; // is this always "CLIPBOARD"?
> // According to ICCCM spec 2.7.2 INCR Properties, and xsel reference
> - XSelectInput (x_display, requestor_win, PropertyChangeMask);
> + XSelectInput(x_display, requestor_win, PropertyChangeMask);
> XDeleteProperty(x_display, requestor_win, prop);
> waiting_for_property_notify = true;
> {
> @@ -2347,31 +2396,32 @@ static void root_win_proc(XEvent& event)
> }
> {
> Lock lock(clipboard_lock);
> - update_clipboard(++size, data);
> + update_clipboard(size, data);
> }
> + clipboard_listener->on_clipboard_notify(clipboard_request_type, clipboard_data,
> + clipboard_data_size);
> + clipboard_request_type = 0;
> XFree(data);
> - clipboard_listener->on_clipboard_change();
> break;
> }
> - case PropertyNotify:
> - if (!waiting_for_property_notify) {
> + case PropertyNotify: {
> + if (!waiting_for_property_notify || event.xproperty.state != PropertyNewValue) {
> break;
> }
> + bool finished_incr = false;
> {
> - if (event.xproperty.state != PropertyNewValue) break;
> - bool finished_incr = false;
> - {
> - Lock lock(clipboard_lock);
> - finished_incr = !get_append_clipboard_data(&event.xselection);
> - }
> - if (finished_incr) {
> - waiting_for_property_notify = false;
> - XDeleteProperty(x_display, event.xselection.requestor,
> - clipboard_prop);
> - clipboard_listener->on_clipboard_change();
> - }
> + Lock lock(clipboard_lock);
> + finished_incr = !get_append_clipboard_data(&event.xselection);
> + }
> + if (finished_incr) {
> + waiting_for_property_notify = false;
> + XDeleteProperty(x_display, event.xselection.requestor, clipboard_prop);
> + clipboard_listener->on_clipboard_notify(clipboard_request_type, clipboard_data,
> + clipboard_data_size);
> + clipboard_request_type = 0;
> }
> break;
> + }
> default:
> return;
> }
> @@ -2463,6 +2513,15 @@ static void init_xrender()
> XRenderQueryVersion(x_display,&major,&minor)&& (major> 0 || minor>= 5);
> }
>
> +static void init_xfixes()
> +{
> + int major;
> + int minor;
> +
> + using_xfixes_1_0 = XFixesQueryExtension(x_display,&xfixes_event_base,&xfixes_error_base)&&
> + XFixesQueryVersion(x_display,&major,&minor)&& major>= 1;
> +}
> +
> unsigned int get_modifier_mask(KeySym modifier)
> {
> int mask = 0;
> @@ -2673,6 +2732,7 @@ void Platform::init()
> init_kbd();
> init_xrandr();
> init_xrender();
> + init_xfixes();
> init_XIM();
>
> struct sigaction act;
> @@ -3009,22 +3069,35 @@ LocalCursor* Platform::create_default_cursor()
> return new XDefaultCursor();
> }
>
> -void Platform::set_clipboard_listener(ClipboardListener* listener)
> +bool Platform::set_clipboard_owner(uint32_t type)
> {
> - //FIXME: XA_CLIPBOARD(x_display)
> - if (XGetSelectionOwner(x_display, XA_PRIMARY) == None) {
> - return;
> + Lock lock(clipboard_lock);
> + uint32_t format = get_clipboard_format(type);
> +
> + if (!format) {
> + LOG_INFO("Unsupported clipboard type %u", type);
> + return false;
> }
> - clipboard_listener = listener;
> - XConvertSelection(x_display, XA_PRIMARY, utf8_atom, clipboard_prop,
> - platform_win, CurrentTime);
> + clipboard_changer = true;
> + clipboard_data_size = 0;
> + XSetSelectionOwner(x_display, XA_PRIMARY, platform_win, CurrentTime);
> + return true;
> +}
> +
> +void Platform::set_clipboard_listener(ClipboardListener* listener)
> +{
> + clipboard_listener = listener ? listener :&default_clipboard_listener;
> }
>
> bool Platform::set_clipboard_data(uint32_t type, const uint8_t* data, int32_t size)
> {
> Lock lock(clipboard_lock);
> + uint32_t format = get_clipboard_format(type);
>
> - LOG_INFO("type %u size %u data %s", type, size, data);
> + if (!format) {
> + LOG_INFO("Unsupported clipboard type %u", type);
> + return false;
> + }
> if (size> clipboard_data_space) {
> delete clipboard_data;
> clipboard_data = new uint8_t[size];
> @@ -3032,22 +3105,33 @@ bool Platform::set_clipboard_data(uint32_t type, const uint8_t* data, int32_t si
> }
> memcpy(clipboard_data, data, size);
> clipboard_data_size = size;
> - //FIXME: XA_CLIPBOARD(x_display)
> - XSetSelectionOwner(x_display, XA_PRIMARY, platform_win, CurrentTime);
> - LOG_INFO("XSetSelectionOwner");
> + clipboard_data_type = type;
> + send_selection_notify(format);
> return true;
> }
>
> -bool Platform::get_clipboard_data(uint32_t type, uint8_t* data, int32_t size)
> +bool Platform::request_clipboard_notification(uint32_t type)
> {
> - //FIXME: check type
> - memcpy(data, clipboard_data, size);
> + uint32_t format = get_clipboard_format(type);
> +
> + if (!format) {
> + LOG_INFO("Unsupported clipboard type %u", type);
> + return false;
> + }
> + if (XGetSelectionOwner(x_display, XA_PRIMARY) == None) {
> + LOG_INFO("No owner for the selection");
> + return false;
> + }
> + if (clipboard_request_type) {
> + LOG_INFO("XConvertSelection request is already pending");
> + return false;
> + }
> + clipboard_request_type = type;
> + XConvertSelection(x_display, XA_PRIMARY, format, clipboard_prop, platform_win, CurrentTime);
> return true;
> }
>
> -int32_t Platform::get_clipboard_data_size(uint32_t type)
> +void Platform::release_clipboard()
> {
> - //FIXME: check type
> - return clipboard_data_size;
> + XSetSelectionOwner(x_display, XA_PRIMARY, None, CurrentTime);
> }
> -
> diff --git a/configure.ac b/configure.ac
> index 3369dde..77fdf6e 100644
> --- a/configure.ac
> +++ b/configure.ac
> @@ -189,9 +189,10 @@ AC_SUBST(GL_LIBS)
> SPICE_NONPKGCONFIG_LIBS+=" $GL_LIBS"
>
> PKG_CHECK_MODULES(XRANDR, xrandr)
> +PKG_CHECK_MODULES(XFIXES, xfixes)
> AC_SUBST(XRANDR_CFLAGS)
> AC_SUBST(XRANDR_LIBS)
> -SPICE_REQUIRES+=" xrandr"
> +SPICE_REQUIRES+=" xrandr xfixes"
>
> PKG_CHECK_MODULES(XRANDR12,
> xrandr>= 1.2,
Regards,
Hans
More information about the Spice-devel
mailing list