Hi
On Thu, Aug 23, 2018 at 9:51 AM Frediano Ziglio <fziglio@xxxxxxxxxx> wrote:
>
> >
> > Hi
> >
> > On Wed, Aug 22, 2018 at 11:11 PM Jonathon Jongsma <jjongsma@xxxxxxxxxx>
> > wrote:
> > >
> > > On Thu, 2018-08-16 at 18:26 +0200, Lukáš Hrázký wrote:
> > > > The version 2 is using a (channel_id, monitor_id) pair to uniquely
> > > > identify the display on which the event occured, instead of the
> > > > ambiguous display_id.
> > > >
> > > > Signed-off-by: Lukáš Hrázký <lhrazky@xxxxxxxxxx>
> > > > ---
> > > > common/messages.h | 8 ++++++++
> > > > spice.proto | 8 ++++++++
> > > > 2 files changed, 16 insertions(+)
> > > >
> > > > diff --git a/common/messages.h b/common/messages.h
> > > > index 942ba07..9b05cee 100644
> > > > --- a/common/messages.h
> > > > +++ b/common/messages.h
> > > > @@ -460,6 +460,14 @@ typedef struct SpiceMsgcMousePosition {
> > > > uint8_t display_id;
> > > > } SpiceMsgcMousePosition;
> > > >
> > > > +typedef struct SpiceMsgcMousePositionV2 {
> > > > + uint32_t x;
> > > > + uint32_t y;
> > > > + uint32_t buttons_state;
> > > > + uint32_t channel_id;
> > > > + uint32_t monitor_id;
> > > > +} SpiceMsgcMousePositionV2;
> > > > +
> > > > typedef struct SpiceMsgcMousePress {
> > > > int32_t button;
> > > > int32_t buttons_state;
> > > > diff --git a/spice.proto b/spice.proto
> > > > index 80976d4..14475fc 100644
> > > > --- a/spice.proto
> > > > +++ b/spice.proto
> > > > @@ -1092,6 +1092,14 @@ channel InputsChannel : BaseChannel {
> > > > uint8 display_id;
> > > > } @ctype(SpiceMsgcMousePosition) mouse_position;
> > > >
> > > > + message {
> > > > + uint32 x;
> > > > + uint32 y;
> > > > + mouse_button_mask buttons_state;
> > > > + uint32 channel_id;
> > > > + uint32 monitor_id;
>
> uint8 for both channel_id and monitor_id.
>
> > > > + } @ctype(SpiceMsgcMousePositionV2) mouse_position_v2;
> > > > +
> > > > message {
> > > > mouse_button button;
> > > > mouse_button_mask buttons_state;
> > >
> > >
> > > This protocol change is clearly necessary if we're going to support any
> > > configurations other than those that we've supported in the past.
> >
> > It's not clear to me that it's necessary if we still have the same
> > constrains, that allow us to use the "displayid = channeld_id ||
> > monitor_id" formula.
>
> The formula is not "channel_id || monitor_id", this is just confusing, is
> more "channel_id + monitor_id" or using GCC extension "channel_id ?: monitor_id".
"channel_id + monitor_id" is wrong: we don't support the combination
with channel_id != 0.
> Note that this formula is present in spice-gtk, virt-viewer (as Lukash mentioned),
> and in SPICE protocol as index in agent MonitorsConfig (structure that is
> used also by the server) and agent MouseState (as display_id field).
> One of the main reason of these patches is that before (as use cases were limited)
> the agent understand what the display_id was, now this is no more true.
This is not a good argument. We can reason and agree about the
protocol, and fix the agent/guest implementation.
>
> Note also that under Linux currently all these protocol limitations allows
> to support only a single device with multiple output, which is a problem for
On linux, a single device with multiple monitors (which is
recommended), or multiple devices (the QXL & Windows way).
> vGPU. Under Windows we support multiple devices but all devices are limited
> to one monitor. Extending Linux using the same limitation requires currently
> that all monitor output would be mapped to a single DisplayChannel surface
> (as currently only surface 0 can be primary) with all monitor sub-area of
> the single surface. The complexity of this just for the above formula is not
> worth. Extending the Windows idea (also in Linux) would require to have single
> monitor for each DisplayChannel (so QXL devices has to be limited to one output)
> so potentially we need to map a single device (like vGPU) to multiple
> DisplayChannels (each with one monitor). This I think would be the only option
> to carry on the above formula (which I state again is part of the protocol).
> Potentially is possible to allocate in advance enough DisplayChannels (on the
> server) to make possible to bind/unbind the outputs dynamically. This last
> however won't solve the agent not knowing what a given display_id is and
> would require some protocol updates (but not between client and server).
> I think Jonathon, whom is mainly working on multi monitor support, was
> thinking on not using these mappings but instead using a single DisplayChannel
> for a single vGPU device even with multiple outputs.
> Personally I would remove the restriction (mainly implementation, not
> protocol) of having the primary surface only with surface_id == 0, this
> was discussed also with Gerd when atomic settings were implemented in Linux
> driver.
That doesn't change the argument about MonitorConfig display_id vs
channel_id & monitor_id.
The only reason I have so far is that the agent is not able to map a
display_id to the correct channel/monitor. Yet, I don't understand why
and why the protocol would need to change.
--
Marc-André Lureau
_______________________________________________
Spice-devel mailing list
Spice-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/spice-devel