[Spice-devel] [PATCH spice-gtk 1/2] channel-display: Make monitors array contain monitors in id order

[Hans de Goede]

Hi,

On 01/16/2013 03:47 PM, Marc-André Lureau wrote:
> On Wed, Jan 16, 2013 at 3:35 PM, Hans de Goede <hdegoede at redhat.com> wrote:
>> Please, please stop being so damn stubborn, and listen to what other
>> people have to say once in a while.
>
> You are not reading my arguments and the code. Please take time to
> read and understand too.

I have to admit that I was misreading the virt-viewer code, my apologies.

Indeed the virtviewer code upon a third careful reading already deals properly
with the monitors property of the display channel even when it contains entries
where monitors[i]->id == i is not true.

That sorts out this bit:

 >> virt-viewer-session-spice.c: virt_viewer_session_spice_display_monitors():
 >>
 >>          display = g_ptr_array_index(displays, monitor->id);

And is my bad, again sorry!

After I finally realized that the virt-viewer code is fine, Marc-André and I
had a long discussion on irc, about the other problematic part, namely this:

>> "
>> spice-widget.c: update_monitor_area()
>>
>>      c = &g_array_index(monitors, SpiceDisplayMonitorConfig, d->monitor_id);
>>
>>
>> "

Which, means that SpiceDisplay->monitor_id == SpiceDisplayMonitorConfig->id
is not true for sparse configs. Which is what caused me to write that, that
code assumes monitors[i]->id == i. Which is the start of all our confusion.
The confusion was that to Marc-André
SpiceDisplay->monitor_id == SpiceDisplayMonitorConfig->id
not always being true was a given and a design choice, where I saw it as
a bug, after that all we had was one big miss-communication :|

Long story short, with the agent getting support for sparse monitor configs
SpiceDisplay->monitor_id == SpiceDisplayMonitorConfig->id not always being
true becomes a problem, as remote-viewer does expect it to be always true.

Since this seems a quite reasonable thing to expect, I'm going to write a
(different) patch tomorrow to make this always be true. And then we should
be good to go and move forward with the agent sparse monitor config support.

Regards,

Hans