Hi,

On 09/26/2010 10:15 AM, Arnon Gilboa wrote:
So far, the clipboard support was disabled in the agent side, so I see no real need for 
supporting the two different clipboard flows and complicating the behavior in both sides. I 
suggest removing the support for the "old clipboard" (VD_AGENT_CAP_CLIPBOARD, which 
was unusable anyway), and using something like VD_AGENT_CAP_CLIPBOARD_BY_DEMAND for the new 
capability & its conditional behavior in both sides.


Right, support for the old way can be completely removed. But since it was 
shipped (short of) we should
use a new capability to check for the new way of handling the clipboard, and 
make both sided only do clipboard
stuff when the other side reports VD_AGENT_CAP_CLIPBOARD_BY_DEMAND. I like that 
as a CAP name btw :)

Regards,

Hans

Hans de Goede wrote:
Hi,

See comments inline.

On 09/21/2010 07:16 PM, Arnon Gilboa wrote:
-enable the clipboard support
-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)
-bonus (no cost): support image cut& paste, currently only with win client
---
vdagent/vdagent.cpp | 323 ++++++++++++++++++++++++++++++++++++++++-----------
1 files changed, 253 insertions(+), 70 deletions(-)

diff --git a/vdagent/vdagent.cpp b/vdagent/vdagent.cpp
index 8ef1a68..5ff595f 100644
--- a/vdagent/vdagent.cpp
+++ b/vdagent/vdagent.cpp
@@ -20,12 +20,21 @@
#include "display_setting.h"
#include<lmcons.h>

-//#define CLIPBOARD_ENABLED
-
#define VD_AGENT_LOG_PATH TEXT("%svdagent.log")
#define VD_AGENT_WINCLASS_NAME TEXT("VDAGENT")
#define VD_INPUT_INTERVAL_MS 20
#define VD_TIMER_ID 1
+#define VD_CLIPBOARD_TIMEOUT_MS 10000
+
+typedef struct VDClipboardFormat {
+ uint32_t format;
+ uint32_t type;
+} VDClipboardFormat;
+
+VDClipboardFormat supported_clipboard_formats[] = {
+ {CF_UNICODETEXT, VD_AGENT_CLIPBOARD_UTF8_TEXT},
+ {CF_DIB, VD_AGENT_CLIPBOARD_BITMAP},

I've no idea what exactly the contents of a DIB buffer are, but
using a windows specific format for image buffers seems very wrong to
me. We should use something standard here (say png) and use that.

<snip>

@@ -562,9 +609,7 @@ bool VDAgent::send_announce_capabilities(bool request)
VD_AGENT_SET_CAPABILITY(caps->caps, VD_AGENT_CAP_MOUSE_STATE);
VD_AGENT_SET_CAPABILITY(caps->caps, VD_AGENT_CAP_MONITORS_CONFIG);
VD_AGENT_SET_CAPABILITY(caps->caps, VD_AGENT_CAP_REPLY);
-#ifdef CLIPBOARD_ENABLED
VD_AGENT_SET_CAPABILITY(caps->caps, VD_AGENT_CAP_CLIPBOARD);
-#endif
VD_AGENT_SET_CAPABILITY(caps->caps, VD_AGENT_CAP_DISPLAY_CONFIG);
vd_printf("sending capabilities:");
for (uint32_t i = 0 ; i< caps_size; ++i) {

Given that the 0.6.0 spice client sets VD_AGENT_CAP_CLIPBOARD, but only
supports the old way of dealing with the clipboard, I believe that we
should introduce a new capability for the new way of dealing with
the clipboard and set / check for that.

<snip>

-//FIXME: currently supports text only
+bool VDAgent::write_message(uint32_t type, uint32_t size = 0, void* data = 
NULL)
+{
+ VDPipeMessage* pipe_msg;
+ VDAgentMessage* msg;
+
+ pipe_msg = (VDPipeMessage*)write_lock(VD_MESSAGE_HEADER_SIZE + size);
+ if (!pipe_msg) {
+ return false;
+ }
+ pipe_msg->type = VD_AGENT_COMMAND;
+ pipe_msg->opaque = VDP_CLIENT_PORT;
+ pipe_msg->size = sizeof(VDAgentMessage) + size;
+ msg = (VDAgentMessage*)pipe_msg->data;
+ msg->protocol = VD_AGENT_PROTOCOL;
+ msg->type = type;
+ msg->opaque = 0;
+ msg->size = size;
+ if (size&& data) {
+ memcpy(msg->data, data, size);
+ }
+ write_unlock(VD_MESSAGE_HEADER_SIZE + size);
+ if (!_pending_write) {
+ write_completion(0, 0,&_pipe_state.write.overlap);
+ }
+ return true;
+}
+
bool VDAgent::on_clipboard_change()
{
- UINT format = CF_UNICODETEXT;
+ uint32_t type = 0;
+
+ for (VDClipboardFormat* iter = supported_clipboard_formats; iter->format&& 
!type; iter++) {
+ if (IsClipboardFormatAvailable(iter->format)) {
+ type = iter->type;
+ }
+ }
+ if (!type) {
+ vd_printf("Unsupported clipboard format");
+ return false;
+ }
+ VDAgentClipboardGrab grab = {type};
+ return write_message(VD_AGENT_CLIPBOARD_GRAB, sizeof(grab),&grab);
+}

Sending of this message should be conditional on the (new) clipboard capability.

+// 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.
+bool VDAgent::on_clipboard_render(UINT format)
+{
+ uint32_t type = get_clipboard_type(format);
+
+ if (!type) {
+ vd_printf("Unsupported clipboard format %u", format);
+ return false;
+ }
+ VDAgentClipboardRequest request = {type};

Sending of this message should be conditional on the (new) clipboard 
capability. Note
I may have missed a few other places where the same goes.

Regards,

Hans

_______________________________________________
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/spice-devel

Reply via email to