[Spice-devel] [PATCH spice-protocol] Add unix GL scanout messages

[Marc-André Lureau]

Hi

----- Original Message -----
> > 
> > Add 2 new messages to the display channel to stream pre-rendered GL
> > images of the display. This is only possible when the client supports
> > SPICE_DISPLAY_CAP_GL_SCANOUT capability.
> > 
> > The first message, SPICE_MSG_DISPLAY_GL_SCANOUT_UNIX, sends a gl image
> > file handle via socket ancillary data, and can be imported in a GL
> > context with the help of eglCreateImageKHR() (as with the 2d canvas, the
> > SPICE_MSG_DISPLAY_MONITORS_CONFIG will give the monitors
> > coordinates (x/y/w/h) within the image). There can be only one scanount
> > per display channel.
> > 
> > A SPICE_MSG_DISPLAY_GL_DRAW message is sent with the coordinate of the
> > region within the scanount to (re)draw on the client display. For each
> > draw, once the client is done with the rendering, it must acknowldge it
> > by sending a SPICE_MSGC_DISPLAY_GL_DRAW_DONE message, in order to
> > release the context (it is expected to improve this in the future with a
> > cross-process GL fence).
> > 
> > The relation with the existing display channel messages is that all
> > other messages are unchanged: the last drawing command received must be
> > displayed. However the scanout display is all or nothing. Consequently,
> > if a 2d canvas draw is received, the display must be switched to the
> > drawn canvas. In other words, if the last message received is a GL draw
> > the display should switch to the GL display, if it's a 2d draw message
> > the display should be switched to the client 2d canvas.
> > 
> > (there will probably be a stipped-down "gl-only" channel in the future,
> > or support for other streaming methods, but this protocol change should
> > be enough for basic virgl or other gpu-accelerated support)
> > 
> > Signed-off-by: Marc-André Lureau <marcandre.lureau at redhat.com>
> > ---
> > v2->v3:
> >  - change flags8 for flags32 (Frediano Ziglio)
> > 
> > spice.proto      | 25 ++++++++++++++++++++++++-
> >  spice/enums.h    |  9 +++++++++
> >  spice/protocol.h |  1 +
> >  3 files changed, 34 insertions(+), 1 deletion(-)
> > 
> > diff --git a/spice.proto b/spice.proto
> > index 3bca900..172d6e3 100644
> > --- a/spice.proto
> > +++ b/spice.proto
> > @@ -678,6 +678,10 @@ struct Head {
> >      uint32 flags;
> >  };
> >  
> > +flags32 gl_scanout_flags {
> > +    Y0TOP
> > +};
> > +
> >  channel DisplayChannel : BaseChannel {
> >   server:
> >      message {
> > @@ -915,7 +919,23 @@ channel DisplayChannel : BaseChannel {
> >          uint32 timeout_ms;
> >      } stream_activate_report;
> >  
> > - client:
> > +    message {
> > +        unix_fd fd;
> > +        uint32 width;
> > +        uint32 height;
> > +        uint32 stride;
> > +        uint32 format; /* drm fourcc */
> > +        gl_scanout_flags flags;
> > +    } gl_scanout_unix;
> > +
> 
> I think names like drm_dma_buf_fd and drm_fourcc_format are much clearer
> (other fields are fine).
> 
> I would like to see something like
> 
>    message {
>        /* dma buffer exported by the Linux DRM layer */
>        unix_fd drm_dma_buf_fd;
>        uint32 width;
>        uint32 height;
>        uint32 stride;
>        /* specifies the format of drm_dma_buf_fd defined in drm_fourcc.h */
>        uint32 drm_fourcc_format;
>        gl_scanout_flags flags;
>    } gl_scanout_unix;

Oops, I forgot that from the v2 thread. Fixed.

> 
> > +    message {
> > +        uint32 x;
> > +        uint32 y;
> > +        uint32 w;
> > +        uint32 h;
> > +    } gl_draw;
> > +
> 
> Why not "Rect box;" ?

As I explained in v2, Rect is quite weird (bottom/top/left/right) and misleading (negative values etc).

> > +client:
> >      message {
> >  	uint8 pixmap_cache_id;
> >  	int64 pixmap_cache_size; //in pixels
> > @@ -937,6 +957,9 @@ channel DisplayChannel : BaseChannel {
> >      message {
> >          image_compression image_compression;
> >      } preferred_compression;
> > +
> > +    message {
> > +    } gl_draw_done;
> >  };
> >  
> >  flags16 keyboard_modifier_flags {
> > diff --git a/spice/enums.h b/spice/enums.h
> > index 16885ac..613db52 100644
> > --- a/spice/enums.h
> > +++ b/spice/enums.h
> > @@ -303,6 +303,12 @@ typedef enum SpiceResourceType {
> >      SPICE_RESOURCE_TYPE_ENUM_END
> >  } SpiceResourceType;
> >  
> > +typedef enum SpiceGlScanoutFlags {
> > +    SPICE_GL_SCANOUT_FLAGS_Y0TOP = (1 << 0),
> > +
> > +    SPICE_GL_SCANOUT_FLAGS_MASK = 0x1
> > +} SpiceGlScanoutFlags;
> > +
> >  typedef enum SpiceKeyboardModifierFlags {
> >      SPICE_KEYBOARD_MODIFIER_FLAGS_SCROLL_LOCK = (1 << 0),
> >      SPICE_KEYBOARD_MODIFIER_FLAGS_NUM_LOCK = (1 << 1),
> > @@ -503,6 +509,8 @@ enum {
> >      SPICE_MSG_DISPLAY_MONITORS_CONFIG,
> >      SPICE_MSG_DISPLAY_DRAW_COMPOSITE,
> >      SPICE_MSG_DISPLAY_STREAM_ACTIVATE_REPORT,
> > +    SPICE_MSG_DISPLAY_GL_SCANOUT_UNIX,
> 
> Sometimes I think SPICE_MSG_DISPLAY_GL_SCANOUT_LINUX would be
> better in this case. dma_buf is really specific.
> But I can live with _UNIX too.

or DMA_BUF? I think I thought _UNIX postfix could help to identify messages that are only valid of unix socket.

> 
> > +    SPICE_MSG_DISPLAY_GL_DRAW,
> >  
> >      SPICE_MSG_END_DISPLAY
> >  };
> > @@ -511,6 +519,7 @@ enum {
> >      SPICE_MSGC_DISPLAY_INIT = 101,
> >      SPICE_MSGC_DISPLAY_STREAM_REPORT,
> >      SPICE_MSGC_DISPLAY_PREFERRED_COMPRESSION,
> > +    SPICE_MSGC_DISPLAY_GL_DRAW_DONE,
> >  
> >      SPICE_MSGC_END_DISPLAY
> >  };
> > diff --git a/spice/protocol.h b/spice/protocol.h
> > index 0c265ee..3e6c624 100644
> > --- a/spice/protocol.h
> > +++ b/spice/protocol.h
> > @@ -136,6 +136,7 @@ enum {
> >      SPICE_DISPLAY_CAP_STREAM_REPORT,
> >      SPICE_DISPLAY_CAP_LZ4_COMPRESSION,
> >      SPICE_DISPLAY_CAP_PREF_COMPRESSION,
> > +    SPICE_DISPLAY_CAP_GL_SCANOUT,
> >  };
> >  
> >  enum {
> > --
> > 2.5.0
> 
> Frediano
> _______________________________________________
> Spice-devel mailing list
> Spice-devel at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/spice-devel
>