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

[Frediano Ziglio]

> 
> 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;

> +    message {
> +        uint32 x;
> +        uint32 y;
> +        uint32 w;
> +        uint32 h;
> +    } gl_draw;
> +

Why not "Rect box;" ?

> +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.

> +    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