> On 20 Mar 2018, at 12:14, Frediano Ziglio <fziglio@xxxxxxxxxx> wrote: > >>> On 19 Mar 2018, at 11:39, Frediano Ziglio <fziglio@xxxxxxxxxx> wrote: >>> >>>> >>>> On 03/13/2018 08:21 AM, Frediano Ziglio wrote: >>>>> Although not necessary for a single monitor DisplayChannel implementation >>>>> this make the DiisplayChannels more coherent from the client >>>>> point of view. >>>>> >>>>> Signed-off-by: Frediano Ziglio <fziglio@xxxxxxxxxx> >>>>> --- >>>>> server/stream-channel.c | 33 ++++++++++++++++++++++++++++++--- >>>>> 1 file changed, 30 insertions(+), 3 deletions(-) >>>>> >>>>> diff --git a/server/stream-channel.c b/server/stream-channel.c >>>>> index 3a3b733f..a9882d9c 100644 >>>>> --- a/server/stream-channel.c >>>>> +++ b/server/stream-channel.c >>>>> @@ -102,6 +102,7 @@ enum { >>>>> RED_PIPE_ITEM_TYPE_STREAM_DATA, >>>>> RED_PIPE_ITEM_TYPE_STREAM_DESTROY, >>>>> RED_PIPE_ITEM_TYPE_STREAM_ACTIVATE_REPORT, >>>>> + RED_PIPE_ITEM_TYPE_MONITORS_CONFIG, >>>>> }; >>>>> >>>>> typedef struct StreamCreateItem { >>>>> @@ -204,6 +205,26 @@ fill_base(SpiceMarshaller *m, const StreamChannel >>>>> *channel) >>>>> spice_marshall_DisplayBase(m, &base); >>>>> } >>>>> >>>>> +static void >>>>> +marshall_monitors_config(RedChannelClient *rcc, StreamChannel *channel, >>>>> SpiceMarshaller *m) >>>>> +{ >>>>> + struct { >>>>> + SpiceMsgDisplayMonitorsConfig config; >>>>> + SpiceHead head; >>>>> + } msg = { >>>>> + { 1, 1, }, >>>>> + { >>>>> + 0, PRIMARY_SURFACE_ID, >>>> >>>> Hi Frediano, >>>> >>>> Why do you send id=0 ? >>>> >>>> Uri. >>>> >>> >>> Too much IDs :-) >>> These IDs are allocated starting from 0 and are per channel so as there >>> is only a monitor (currently) the ID should be 0. >>> So this id should not be confused with QXL ID or client/guest ID. >> >> If Uri is confused, a comment may be in order. >> >> Why so many IDs? Is it just to avoid an ID lookup during initialization? >> > > Not getting the lookup. I assume we selected different IDs because they access different arrays, so you can do array[id] instead of array[lookup(id)]. > > We have a global idea of the "monitor ID" which is quite confusing and > prone to different issues. The DisplayChannel (protocol) use channel IDs > (allocated per channel type starting from 0) and head IDs (allocated per > channel starting from 0). The client and vdagent use the global monitor ID. > The client craft these monitor IDs using basically the formula > "channel ID ? channel ID : head ID" and assuming the vdagent uses the same > IDs. Currently there are different problems of these assumption/schema. Yes, I can imagine the problems. This is confusing. But you answered the question of what we have, not the “why”. It may be a lookup optimization as indicated above. Or it may be that different IDs really refer to different entities, but then I’m surprised by a computation such as “channelID ? channelID : headID” which mixes two kinds of IDs. If this causes problems, shouldn’t we start by clarifying what a monitor ID should be? >>> >>> Frediano >>> >>>>> + channel->width, channel->height, >>>>> + 0, 0, >>>>> + 0 // flags >>>>> + } >>>>> + }; >>>>> + >>>>> + red_channel_client_init_send_data(rcc, >>>>> SPICE_MSG_DISPLAY_MONITORS_CONFIG); >>>>> + spice_marshall_msg_display_monitors_config(m, &msg.config); >>>>> +} >>>>> + >>>>> static void >>>>> stream_channel_send_item(RedChannelClient *rcc, RedPipeItem *pipe_item) >>>>> { >>>>> @@ -229,6 +250,12 @@ stream_channel_send_item(RedChannelClient *rcc, >>>>> RedPipeItem *pipe_item) >>>>> spice_marshall_msg_display_surface_create(m, &surface_create); >>>>> break; >>>>> } >>>>> + case RED_PIPE_ITEM_TYPE_MONITORS_CONFIG: >>>>> + if (!red_channel_client_test_remote_cap(rcc, >>>>> SPICE_DISPLAY_CAP_MONITORS_CONFIG)) { >>>>> + return; >>>>> + } >>>>> + marshall_monitors_config(rcc, channel, m); >>>>> + break; >>>>> case RED_PIPE_ITEM_TYPE_SURFACE_DESTROY: { >>>>> red_channel_client_init_send_data(rcc, >>>>> SPICE_MSG_DISPLAY_SURFACE_DESTROY); >>>>> SpiceMsgSurfaceDestroy surface_destroy = { PRIMARY_SURFACE_ID }; >>>>> @@ -397,7 +424,6 @@ stream_channel_connect(RedChannel *red_channel, >>>>> RedClient *red_client, RedStream >>>>> request_new_stream(channel, start); >>>>> >>>>> >>>>> - // TODO set capabilities like SPICE_DISPLAY_CAP_MONITORS_CONFIG >>>>> // see guest_set_client_capabilities >>>>> RedChannelClient *rcc = RED_CHANNEL_CLIENT(client); >>>>> red_channel_client_push_set_ack(rcc); >>>>> @@ -415,6 +441,7 @@ stream_channel_connect(RedChannel *red_channel, >>>>> RedClient *red_client, RedStream >>>>> >>>>> // pass proper data >>>>> red_channel_client_pipe_add_type(rcc, >>>>> RED_PIPE_ITEM_TYPE_SURFACE_CREATE); >>>>> + red_channel_client_pipe_add_type(rcc, >>>>> RED_PIPE_ITEM_TYPE_MONITORS_CONFIG); >>>>> // surface data >>>>> red_channel_client_pipe_add_type(rcc, >>>>> RED_PIPE_ITEM_TYPE_FILL_SURFACE); >>>>> // TODO monitor configs ?? >>>>> @@ -433,8 +460,7 @@ stream_channel_constructed(GObject *object) >>>>> client_cbs.connect = stream_channel_connect; >>>>> red_channel_register_client_cbs(red_channel, &client_cbs, NULL); >>>>> >>>>> - // TODO, send monitor to support multiple monitors in the future >>>>> -// red_channel_set_cap(red_channel, >>>>> SPICE_DISPLAY_CAP_MONITORS_CONFIG); >>>>> + red_channel_set_cap(red_channel, SPICE_DISPLAY_CAP_MONITORS_CONFIG); >>>>> red_channel_set_cap(red_channel, SPICE_DISPLAY_CAP_STREAM_REPORT); >>>>> >>>>> reds_register_channel(reds, red_channel); >>>>> @@ -489,6 +515,7 @@ stream_channel_change_format(StreamChannel *channel, >>>>> const StreamMsgFormat *fmt) >>>>> channel->width = fmt->width; >>>>> channel->height = fmt->height; >>>>> red_channel_pipes_add_type(red_channel, >>>>> RED_PIPE_ITEM_TYPE_SURFACE_CREATE); >>>>> + red_channel_pipes_add_type(red_channel, >>>>> RED_PIPE_ITEM_TYPE_MONITORS_CONFIG); >>>>> // TODO monitors config ?? >>>>> red_channel_pipes_add_empty_msg(red_channel, >>>>> SPICE_MSG_DISPLAY_MARK); >>>>> } >>>>> > > Frediano > _______________________________________________ > Spice-devel mailing list > Spice-devel@xxxxxxxxxxxxxxxxxxxxx > https://lists.freedesktop.org/mailman/listinfo/spice-devel _______________________________________________ Spice-devel mailing list Spice-devel@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/spice-devel