Re: [RFC PATCH spice-common v2 02/20] A version 2 of the MousePosition message

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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;
> > +    } @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.

So do we have to support both multiple display channel & multiple
monitors per display channels? Could you give some details on how this
is going to operate?

Iff it's the case, we will need to modify the protocol.


> One thing I just realized is that a channel id is not enough by itself
> to uniquely identify a channel. You need a channel type and a channel
> id. You could argue that I'm being pedantic and that it's obvious that
> we're talking about display channels here. And maybe you'd be right.
> But maybe it would be better to be more explicit? You could do that
> either by using the existing SpiceChannelId type (which is a pair of
> (type, id)), or by simply using a more descriptive name (e.g.
> display_channel, or display_channel_id).
>
> Or if nobody else cares, we could leave it like this.
>
> Reviewed-by: Jonathon Jongsma <jjongsma@xxxxxxxxxx>
> _______________________________________________
> Spice-devel mailing list
> Spice-devel@xxxxxxxxxxxxxxxxxxxxx
> https://lists.freedesktop.org/mailman/listinfo/spice-devel



-- 
Marc-André Lureau
_______________________________________________
Spice-devel mailing list
Spice-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/spice-devel




[Index of Archives]     [Linux Virtualization]     [Linux Virtualization]     [Linux ARM Kernel]     [Linux ARM]     [Linux Omap]     [Fedora ARM]     [IETF Annouce]     [Security]     [Bugtraq]     [Linux OMAP]     [Linux MIPS]     [ECOS]     [Asterisk Internet PBX]     [Linux API]     [Monitors]