[Spice-devel] [PATCH] spice-protocl: add SPICE_MSG_DISPLAY_SURFACE_CREATE/DESTROY

[Izik Eidus]

On Fri, 19 Feb 2010 14:19:29 +0100
Alexander Larsson <alexl at redhat.com> wrote:

> On Thu, 2010-02-18 at 01:16 +0200, Izik Eidus wrote:
> > From 7a182d48ee603177d522edec9b189f2f6e3ee5a6 Mon Sep 17 00:00:00 2001
> > From: Izik Eidus <ieidus at redhat.com>
> > Date: Thu, 18 Feb 2010 01:13:56 +0200
> > Subject: [PATCH] spice-protocl: add
> > SPICE_MSG_DISPLAY_SURFACE_CREATE/DESTROY
> > 
> > Protocol commands to create/destroy surface on the client.
> > 
> > I removed SPICE_MSG_DISPLAY_MODE as it now not needed
> > 
> > Signed-off-by: Izik Eidus <ieidus at redhat.com>
> > ---
> >  spice/protocol.h |   26 ++++++++++++++++++++++++--
> >  1 files changed, 24 insertions(+), 2 deletions(-)
> > 
> > diff --git a/spice/protocol.h b/spice/protocol.h
> > index 5f01a12..e0dbd32 100644
> > --- a/spice/protocol.h
> > +++ b/spice/protocol.h
> > @@ -339,8 +339,7 @@ typedef struct SPICE_ATTR_PACKED
> > SpiceMsgcMainMouseModeRequest {
> >  } SpiceMsgcMainMouseModeRequest;
> >  
> >  enum {
> > -    SPICE_MSG_DISPLAY_MODE = SPICE_MSG_FIRST_AVAIL,
> > -    SPICE_MSG_DISPLAY_MARK,
> > +    SPICE_MSG_DISPLAY_MARK = SPICE_MSG_FIRST_AVAIL,
> >      SPICE_MSG_DISPLAY_RESET,
> >      SPICE_MSG_DISPLAY_COPY_BITS,
> 
> Is this really necessary? It shifts the value of all the other messages,
> which makes backwards compat unnecessarily complex. Can't we just leave
> the old value unused?


While I was aware of this, I thought that it isnt the end of the world,

I dont mind keeping it around as unused value, we can probably kick it
later.

> 
> > @@ -368,6 +367,9 @@ enum {
> >      SPICE_MSG_DISPLAY_DRAW_TRANSPARENT,
> >      SPICE_MSG_DISPLAY_DRAW_ALPHA_BLEND,
> >  
> > +    SPICE_MSG_DISPLAY_SURFACE_CREATE,
> > +    SPICE_MSG_DISPLAY_SURFACE_DESTROY,
> > +
> >      SPICE_MSG_END_DISPLAY,
> >  };
> >  
> > @@ -389,7 +391,26 @@ typedef struct SPICE_ATTR_PACKED
> > SpiceMsgDisplayMode {
> >      uint32_t bits;
> >  } SpiceMsgDisplayMode;
> >  
> > +enum {
> > +    SPICE_SURFACE_FLAGS_PRIMARY = (1 << 0),
> > +};
> > +
> > +typedef struct SPICE_ATTR_PACKED SpiceMsgSurfaceCreate {
> > +    uint32_t surface_id;
> > +    uint32_t width;
> > +    uint32_t height;
> > +    int32_t stride;
> > +    uint8_t depth;
> > +    uint32_t flags;
> > +    uint32_t type;
> > +} SpiceMsgSurfaceCreate;
> 
> Do you really need for the surface to have a specific stride? What does
> the server care what the stride is on the client?


This would always be (as for now) width == stride,
But as we dont know how the future will lead us with what kind of requiments
is it really that bad to give stride as well?

(I really have no strong feeling to drop it,
 so if you still want, i will drop it)


> 
> Also, what happens if you create multiple PRIMARY surfaces? Is that how
> multi-monitor works?

Right now each monitor have it own worker, but when we will support dual-head
monitors (like X need) then the answer is yes, there would be multiple PRIMARYs.

Thanks.

> 
>