[Spice-devel] [PATCH spice-gtk 2/2] channel-main: Add support for VD_AGENT_CAP_SPARSE_MONITORS_CONFIG (rhbz#881072)

[Hans de Goede]

Hi,

On 01/11/2013 09:02 PM, Marc-André Lureau wrote:
> Hi
>
> My understanding of this patch is that you send all monitors config,
> regardless if there are disabled. This is a bit wasteful, but not a
> real problem.
>
> Why is that needed if the implementation handle "missing" monitors id
> as disabled? (which is what the current implementation should do, no?)

This code builds a VDAgentMonitorsConfig which contains
a "struct VDAgentMonConfig monitors[]" array and the
VDAgentMonConfig struct does not contain ids, instead the expectation is
that the info for the display with id x is stored at monitors[x]

Note that this being a problem is already acknowledged in the current
code with a FIXME comment, which this patch actually fixes.

Regards,

Hans




>
> On Thu, Jan 10, 2013 at 11:52 PM, Hans de Goede <hdegoede at redhat.com> wrote:
>> Signed-off-by: Hans de Goede <hdegoede at redhat.com>
>> ---
>>   gtk/channel-main.c | 20 ++++++++++++++------
>>   1 file changed, 14 insertions(+), 6 deletions(-)
>>
>> diff --git a/gtk/channel-main.c b/gtk/channel-main.c
>> index 653b989..720fcfa 100644
>> --- a/gtk/channel-main.c
>> +++ b/gtk/channel-main.c
>> @@ -939,11 +939,15 @@ gboolean spice_main_send_monitor_config(SpiceMainChannel *channel)
>>       c = channel->priv;
>>       g_return_val_if_fail(c->agent_connected, FALSE);
>>
>> -    monitors = 0;
>> -    /* FIXME: fix MonitorConfig to be per display */
>> -    for (i = 0; i < SPICE_N_ELEMENTS(c->display); i++) {
>> -        if (c->display[i].enabled)
>> -            monitors += 1;
>> +    if (spice_main_agent_test_capability(channel,
>> +                                     VD_AGENT_CAP_SPARSE_MONITORS_CONFIG)) {
>> +        monitors = SPICE_N_ELEMENTS(c->display);
>> +    } else {
>> +        monitors = 0;
>> +        for (i = 0; i < SPICE_N_ELEMENTS(c->display); i++) {
>> +            if (c->display[i].enabled)
>> +                monitors += 1;
>> +        }
>>       }
>>
>>       size = sizeof(VDAgentMonitorsConfig) + sizeof(VDAgentMonConfig) * monitors;
>> @@ -956,8 +960,12 @@ gboolean spice_main_send_monitor_config(SpiceMainChannel *channel)
>>
>>       j = 0;
>>       for (i = 0; i < SPICE_N_ELEMENTS(c->display); i++) {
>> -        if (!c->display[i].enabled)
>> +        if (!c->display[i].enabled) {
>> +            if (spice_main_agent_test_capability(channel,
>> +                                     VD_AGENT_CAP_SPARSE_MONITORS_CONFIG))
>> +                j++;
>>               continue;
>> +        }
>>           mon->monitors[j].depth  = c->display_color_depth ? c->display_color_depth : 32;
>>           mon->monitors[j].width  = c->display[j].width;
>>           mon->monitors[j].height = c->display[j].height;
>> --
>> 1.8.0.2
>>
>> _______________________________________________
>> Spice-devel mailing list
>> Spice-devel at lists.freedesktop.org
>> http://lists.freedesktop.org/mailman/listinfo/spice-devel
>
>
>