Hi
Why not be consistant with QXLMonitorsConfig and add an id field to
VDAgentMonitorConfig? That's why spice-gtk didn't care much about
QXLMonitorsConfig id, since VDAgentMonitorConfig didn't had one (since
it was meant to reflect real hw I suppose), it couldn't set it back
(unless the agent learned to deal with "sparse" monitors in some
ways). I agree it could have been done in the first place. Patch looks
good otherwise.
On Fri, Jan 11, 2013 at 10:55 PM, Hans de Goede <hdegoede@xxxxxxxxxx> wrote:
> 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@xxxxxxxxxx>
>> wrote:
>>>
>>> Signed-off-by: Hans de Goede <hdegoede@xxxxxxxxxx>
>>> ---
>>> 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@xxxxxxxxxxxxxxxxxxxxx
>>> http://lists.freedesktop.org/mailman/listinfo/spice-devel
>>
>>
>>
>>
>
--
Marc-André Lureau
_______________________________________________
Spice-devel mailing list
Spice-devel@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/spice-devel