On Wed, 2018-08-22 at 16:11 -0500, Jonathon Jongsma 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.
>
> 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).
Well, it's only display channels that currently have any monitors.
You're technically right, if we ever introduce another channel type
similar to a display channel that will also have monitors, we would
need this distinction. (I don't think we'll ever want to add monitors
to any of the existing channels)
I feel like it's very unlikely and am inclined not to add the type, but
yeah...
> 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