! Please note that this is a snapshot of our old Bugzilla server, which is read only since May 29, 2020. Please go to gitlab.xfce.org for our new server !
Display settings mixes up monitor EDID names
Status:
RESOLVED: FIXED
Product:
Xfce4-settings
Component:
Display Settings

Comments

Description Alistair Buxton 2014-02-27 20:08:16 CET
in xfce_randr_populate is this piece of code:

    /* prepare the temporary cache */
    outputs = g_ptr_array_new ();

    /* walk the outputs */
    for (n = 0; n < randr->priv->resources->noutput; ++n)
    {
        /* get the output info */
        output_info = XRRGetOutputInfo (xdisplay, randr->priv->resources,
                                        randr->priv->resources->outputs[n]);

        /* forget about disconnected outputs */
        if (output_info->connection != RR_Connected)
        {
            XRRFreeOutputInfo (output_info);
            continue;
        }

        /* cache it */
        g_ptr_array_add (outputs, output_info);
    }

    /* migrate the temporary cache */
    randr->noutput = outputs->len;
    randr->priv->output_info = (XRROutputInfo **) g_ptr_array_free (outputs, FALSE);

This gets the info for each output, and if the output is connected it store the info.


Later on is this:

    /* walk the connected outputs */
    for (m = 0; m < randr->noutput; ++m)
    {

   ...
        /* fill in the name used by the UI */
        randr->friendly_name[m] = xfce_randr_friendly_name (randr, m);
   ...

    }


Inside friendly name it eventually does this:

edid_data = xfce_randr_read_edid_data (xdisplay, randr->priv->resources->outputs[output]);

Here, output = m.

This is invalid because m only counts connected outputs, but is being used as an offset into randr->priv->resources->outputs, which includes unconnected outputs. As a result, all the connected monitor names are wrong if you have unconnected displays.

Commenting this piece of code fixes the problem:


        /* forget about disconnected outputs */
        if (output_info->connection != RR_Connected)
        {
            XRRFreeOutputInfo (output_info);
            continue;
        }


However, this causes all unconnected outputs to be listed in the display settings (although they are "greyed" indicating they are not connected).
Comment 1 Alistair Buxton 2014-02-27 21:26:53 CET
So this isn't really a proper fix, but I think it's better than having the output names totally wrong.

A proper fix would be to store the actual output number somewhere so that we can index correctly into the full array. Alternatively, check for which outputs are connected when building the gui, instead of when building the cache. Or maybe even don't use the cache at all and always use the randr supplied data as-is.
Comment 2 Alistair Buxton 2014-02-27 21:28:14 CET
Oh and one other thing: the wrong indexing might be getting used in other parts of the code: I have not checked this, but someone should take a look. The wrong monitor names are just the most obvious visible side effect.
Comment 3 Lionel Le Folgoc 2014-02-27 23:11:30 CET
Thanks for your bug report. I guess that's easy to miss when you only have two outputs when you code. ;-)

The wrong index is only used one more time, at the beginning of the loop:

> /* find the primary screen if supported */
>         if (randr->priv->has_1_3 && XRRGetOutputPrimary (xdisplay, GDK_WINDOW_XID (root_window)) == randr->priv->resources->outputs[m])

(basically any access to randr->priv->resources->outputs)

I don't remember why I used a GPtrArray (I did that too long ago), probably because it's auto-expanding and g_ptr_array_free() returns an array, which was what I needed. But that was probably overkill anyway.

As you wrote, the least intrusive fix (probably better for Xubuntu at this time) is to store the connected RROutputs in a small array during the first loop, so they have the same index as randr->priv->output_info, and so you can replace the two uses of randr->priv->resources->outputs[m] with that_array[m]. And it can be destroyed after the second loop. Anyway, YMMV.

Another possible fix is to drop the temporary cache and allocate arrays of randr->priv->resources->noutput elements. There will be a few unused elements at the end (randr->priv->resources->noutput - randr->noutput), but it's small and we probably don't care.
Comment 5 Simon Steinbeiss editbugs 2014-08-29 15:34:29 CEST
This bug has been fixed in xfce4-settings 4.11.3.

Bug #10717

Reported by:
Alistair Buxton
Reported on: 2014-02-27
Last modified on: 2014-08-29

People

Assignee:
Jérôme Guelfucci
CC List:
5 users

Version

Version:
4.11.0

Attachments

Additional information