qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-devel] [RfC PATCH] vnc: rich cursor support.


From: Anthony Liguori
Subject: Re: [Qemu-devel] [RfC PATCH] vnc: rich cursor support.
Date: Mon, 03 May 2010 07:20:25 -0500
User-agent: Mozilla/5.0 (X11; U; Linux x86_64; en-US; rv:1.9.1.5) Gecko/20091209 Fedora/3.0-4.fc12 Lightning/1.0pre Thunderbird/3.0

On 05/03/2010 06:59 AM, Gerd Hoffmann wrote:
   Hi,

Simple patch.  Difficuilt matter.  Not really sure where to go from
here ...

It'll be a complicated patch :-) I looked at this a while ago and there are a few gotchas.

The whole thing is about local cursor support, i.e. just let the UI
(sdl, vnc viewer, spice client, ...) draw the mouse pointer directly,
without round trip to the guest for mouse pointer rendering.

Current state in upstream qemu:  vmware svga supports it.  sdl supports
it.  When running vmware vga on sdl you should get it.  In theory.  In
practice it doesn't work correctly.

Cirrus technically supports it do but almost nothing uses the hardware cursor support in Cirrus (only win2k configured in a certain way).

SDL can only handle 1bit (black/white) mouse cursors (with mask)

I personally don't think we should even bother with anything other than ARGB cursors. Not enough things render 1bit cursors via hardware in my experience.

I've seen vmware vga send only 1bit cursors (with mask, winxp guest),
althougth it seems to be designed to support colored pointers too.

VMware VGA sends full ARGB cursors. That's what you get by default when you use vmware-vga and X.

vnc has two extentions for it: xcursor (1bit too, using a mask, but also
allows to specify foreground/backgrund color) and rich cursor (uses
pixelformat of the display, i.e. allows color).

The key problem with VNC is that it has no notion of disabling cursor offload. This means that when the guest tries to disable it (like via a reset cycle), we have to make sure to send a NULL cursor to avoid the cursor being rendered.

This effectively disables any attempt by the client to draw a double cursor though.

spice supports a whole bunch of formats, although I've seen only two of
them used in practice:  1bit (with mask) and 32bit (rgb + alpha).


The current hooks supporting local pointer (mouse_set, cursor_define)
are in DisplayState, although I think they belong into
DisplayChangeListener.


Ok, how to sort this mess?


I think we should put everything into a QEMUCursor struct, so we don't
have to pass tons of parameters to the ->cursor_define() callback.

Agreed.

Does it make sense to reuse "struct PixelFormat" for the cursor?  I tend
to think not.  I expect we'll see three different cases be used in
practice:

   (1) 1-bit image (and mask).
   (2) Same pixelformat as DisplayState (and mask).
   (3) 32bit RGB + alpha.

I think always making a cursor 32bit ARGB would be reasonable. Let the backend sort things out. Since we have the PixelFormat structures, it's easy enough to add a routine to convert between formats.

Current PixelFormat can cover only (2) and even in that case it is
redundant with DisplayState->pf.

I think we also better allow only certain sizes.  Minimum requirement
should be that the width is a multiple of 8.  Handling bitmasks just
become too ugly without that.  I'd tend to go further even and allow
only 32x32 and 64x64 mouse pointers.  Maybe 48x48 too.

I'm not sure it's necessary to be so restrictive. If you assume ARGB, then masking isn't an issue.

Regards,

Anthony Liguori

Comments?


cheers,
   Gerd

---
  vnc.c |   35 +++++++++++++++++++++++++++++++++++
  vnc.h |    2 +
  2 files changed, 37 insertions(+), 0 deletions(-)

diff --git a/vnc.c b/vnc.c
index 9ba603c..5d9b9cb 100644
--- a/vnc.c
+++ b/vnc.c
@@ -925,6 +925,36 @@ static void vnc_dpy_copy(DisplayState *ds, int src_x, int 
src_y, int dst_x, int
      }
  }

+static void vnc_mouse_set(int x, int y, int visible)
+{
+    /* can we do that ??? */
+}
+
+static void vnc_cursor_define(int width, int height, int bpp,
+                              int hot_x, int hot_y,
+                              uint8_t *image, uint8_t *mask)
+{
+    VncDisplay *vd = vnc_display;
+    int pixels, isize, msize;
+    VncState *vs;
+
+    pixels = width * height;
+    isize = pixels * bpp / 8;
+    msize = pixels / 8;
+
+    QTAILQ_FOREACH(vs,&vd->clients, next) {
+        if (!vnc_has_feature(vs, VNC_FEATURE_RICH_CURSOR))
+            continue;
+        vnc_write_u8(vs,  VNC_MSG_SERVER_FRAMEBUFFER_UPDATE);
+        vnc_write_u8(vs,  0);  /*  padding     */
+        vnc_write_u16(vs, 1);  /*  # of rects  */
+        vnc_framebuffer_update(vs, hot_x, hot_y, width, height,
+                               VNC_ENCODING_RICH_CURSOR);
+        vnc_write(vs, image, isize);
+        vnc_write(vs, mask, msize);
+    }
+}
+
  static int find_and_clear_dirty_height(struct VncState *vs,
                                         int y, int last_x, int x)
  {
@@ -1800,6 +1830,9 @@ static void set_encodings(VncState *vs, int32_t 
*encodings, size_t n_encodings)
          case VNC_ENCODING_POINTER_TYPE_CHANGE:
              vs->features |= VNC_FEATURE_POINTER_TYPE_CHANGE_MASK;
              break;
+        case VNC_ENCODING_RICH_CURSOR:
+            vs->features |= VNC_FEATURE_RICH_CURSOR_MASK;
+            break;
          case VNC_ENCODING_EXT_KEY_EVENT:
              send_ext_key_event_ack(vs);
              break;
@@ -2487,6 +2520,8 @@ void vnc_display_init(DisplayState *ds)
      dcl->dpy_resize = vnc_dpy_resize;
      dcl->dpy_setdata = vnc_dpy_setdata;
      register_displaychangelistener(ds, dcl);
+    ds->mouse_set = vnc_mouse_set;
+    ds->cursor_define = vnc_cursor_define;
  }


diff --git a/vnc.h b/vnc.h
index b593608..ede9040 100644
--- a/vnc.h
+++ b/vnc.h
@@ -266,6 +266,7 @@ enum {
  #define VNC_FEATURE_TIGHT                    4
  #define VNC_FEATURE_ZLIB                     5
  #define VNC_FEATURE_COPYRECT                 6
+#define VNC_FEATURE_RICH_CURSOR              7

  #define VNC_FEATURE_RESIZE_MASK              (1<<  VNC_FEATURE_RESIZE)
  #define VNC_FEATURE_HEXTILE_MASK             (1<<  VNC_FEATURE_HEXTILE)
@@ -274,6 +275,7 @@ enum {
  #define VNC_FEATURE_TIGHT_MASK               (1<<  VNC_FEATURE_TIGHT)
  #define VNC_FEATURE_ZLIB_MASK                (1<<  VNC_FEATURE_ZLIB)
  #define VNC_FEATURE_COPYRECT_MASK            (1<<  VNC_FEATURE_COPYRECT)
+#define VNC_FEATURE_RICH_CURSOR_MASK         (1<<  VNC_FEATURE_RICH_CURSOR)


  /* Client ->  Server message IDs */





reply via email to

[Prev in Thread] Current Thread [Next in Thread]