On Wed, Jan 16, 2013 at 3:35 PM, Hans de Goede <hdegoede@xxxxxxxxxx> 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.
> "
> spice-widget.c: update_monitor_area()
>
> c = &g_array_index(monitors, SpiceDisplayMonitorConfig, d->monitor_id);
>
>
> virt-viewer-session-spice.c: virt_viewer_session_spice_display_monitors():
>
> display = g_ptr_array_index(displays, monitor->id);
> "
That's exactly what I explained in the previous mail.
>>> that the monitors array can now contain 0x0 sized (iow disabled)
>>> monitors,
>>> remote-viewer already checks for this.
>>
>> Yes, because it builds its own array.
>
> Not true, it gets the monitors property directly from the display channel:
>
> virt-viewer-session-spice.c: virt_viewer_session_spice_display_monitors():
>
> g_object_get(channel,
> "monitors", &monitors,
> "monitors-max", &monitors_max,
> NULL);
>
> And then it creates max_monitors VirtViewerDisplaySpice
> objects, and then walks the monitors array enabling
> any VirtViewerDisplaySpice where width != 0 && height != 0:
>
> for (i = 0; i < monitors->len; i++) {
> SpiceDisplayMonitorConfig *monitor = &g_array_index(monitors,
> SpiceDisp
>
> display = g_ptr_array_index(displays, monitor->id);
> g_return_if_fail(display != NULL);
>
> if (monitor->width == 0 || monitor->width == 0)
> continue;
>
> virt_viewer_display_set_enabled(VIRT_VIEWER_DISPLAY(display), TRUE);
> virt_viewer_display_set_desktop_size(VIRT_VIEWER_DISPLAY(display),
> monitor->width,
> monitor->height);
> }
>
>
> See how it is addressing the monitors property of the display-channel
> by id !! let me single out the line where this happens for you (again):
>
>
> display = g_ptr_array_index(displays, monitor->id);
yes, because the array is "sparse" this time:
g_ptr_array_set_size(displays, monitors_max);
>
>>> Thus this patch:
>>> 1) Makes the monitors[i]->id == i assumption be true
>>
>> No need for changing that.
>
> /me bangs head on table.
>
> There is a need to change that because the monitors property of the display
> channel is a 1 on 1 copy of the monitors message on the wire, and for that
> message monitors[i]->id == i is simply *not* true, yet it is assumed in
> 2 places in the code, as I've pointed out *twice* now!
sorry, your argument is false
--
Marc-André Lureau
_______________________________________________
Spice-devel mailing list
Spice-devel@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/spice-devel