I'm wondering if this new message is strictly necessary. We *could*
still support the vgpu scenario with the old message, couldn't we?
(By the way, I'm not asking whether the old message is *better* or not,
just whether this change is absolutely necessary to support the
scenario we're trying to support.)
As far as I can tell, in the old scenario, the client would send a
request to the server saying basically:
* I want 3 displays.
* The first one should be W1xH1 and located at (X1,Y1)
* The second one should be W2xH2 and located at (X2,Y2)
* The third one should be W3xH3 and located at (X3,Y3)
This message is passed on to the vdagent (let's just consider that case
for now), and the vdagent reconfigures X so that we have 3 displays
that (mostly) satisfy those requirements. There are a few reasons that
the new configuration may not match the request exactly. For example:
* The exact resolution is not supported by the driver
* There is not enough memory to allocate a display that large large
* The given coordinates would cause the displays to overlap, so the
agent may adjust them to be non-overlapping
So it's not unreasonable to expect the guest to make some decisions
about how to achieve the requested configuration.
With your patch, this changes slightly to:
* I want 3 displays
* The first one must be guest output ID1 and should be W1xH1 @
(X1,Y1)
* etc.
Adding this guest_output_id to the message makes things a bit more
explicit and perhaps more predictable, but it doesn't seem like it is
absolutely necessary. In the end, both of these scenarios will result
in the guest reconfiguring the displays to match the request from the
client. There is a chance that they'll use different guest output IDs
to accomplish this configuration, but that is easily handled by the
client, I think.
If my analysis is correct, I wonder if this particular part of the
series is worth keeping, since the benefit we get from it may not be
worth the protocol compatibility issues that it introduces. Thoughts?
Reviewed-by: Jonathon Jongsma <jjongsma@xxxxxxxxxx>
On Thu, 2018-08-16 at 18:26 +0200, Lukáš Hrázký wrote:
> To keep compatibility with old endpoints (any of client, server,
> vd_agent), we need to copy the message to add the guest_output_id
> field.
>
> The guest_output_id is the guest-side id of the xrandr output (to be
> precise, it is the index in the list of xrandr outputs) that is set
> in
> the monitors config messages by the streaming agent. It is later used
> in
> the guest by vd_agent for mouse input and possibly monitors_config
> (enabling/disabling monitors and setting the resolution/position of
> monitors).
>
> Signed-off-by: Lukáš Hrázký <lhrazky@xxxxxxxxxx>
> ---
> spice/vd_agent.h | 27 +++++++++++++++++++++++++++
> 1 file changed, 27 insertions(+)
>
> diff --git a/spice/vd_agent.h b/spice/vd_agent.h
> index dda7044..43ec1a0 100644
> --- a/spice/vd_agent.h
> +++ b/spice/vd_agent.h
> @@ -154,6 +154,33 @@ typedef struct SPICE_ATTR_PACKED
> VDAgentMonitorsConfig {
> VDAgentMonConfig monitors[0];
> } VDAgentMonitorsConfig;
>
> +typedef struct SPICE_ATTR_PACKED VDAgentMonConfigV2 {
> + /* The guest_output_id is the guest-side id of the xrandr output
> (to be
> + * precise, it is the index in the list of xrandr outputs) that
> is set in
> + * the monitors config messages by the streaming agent. It is
> later used in
> + * the guest by vd_agent for mouse input and possibly
> monitors_config
> + * (enabling/disabling monitors and setting the
> resolution/position of
> + * monitors).
> + */
> + uint32_t guest_output_id;
> + /*
> + * Note a width and height of 0 can be used to indicate a
> disabled
> + * monitor, this may only be used with agents with the
> + * VD_AGENT_CAP_SPARSE_MONITORS_CONFIG capability.
> + */
> + uint32_t height;
> + uint32_t width;
> + uint32_t depth;
> + int32_t x;
> + int32_t y;
> +} VDAgentMonConfigV2;
> +
> +typedef struct SPICE_ATTR_PACKED VDAgentMonitorsConfigV2 {
> + uint32_t num_of_monitors;
> + uint32_t flags;
> + VDAgentMonConfigV2 monitors[0];
> +} VDAgentMonitorsConfigV2;
> +
> enum {
> VD_AGENT_DISPLAY_CONFIG_FLAG_DISABLE_WALLPAPER = (1 << 0),
> VD_AGENT_DISPLAY_CONFIG_FLAG_DISABLE_FONT_SMOOTH = (1 << 1),
_______________________________________________
Spice-devel mailing list
Spice-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/spice-devel