monitor names incorrect in dual monitor setup

[David Danforth]

The 'display settings' mis-labels the two monitors connected to my computer. I have an ASUS monitor and a ViewSonic monitor. The ASUS monitor is labeled ViewSonic, and the ViewSonic is labeled HDMI1. This mis-labeling is the only apparent error. 'Display settings' otherwise correctly recognizes the monitor specifications. The monitors are correctly displayed in relationship to one another.

ProblemType: Bug
DistroRelease: Ubuntu 14.04
Package: xfce4-settings 4.11.2-1ubuntu1
ProcVersionSignature: Ubuntu 3.13.0-19.40-generic 3.13.6
Uname: Linux 3.13.0-19-generic x86_64
ApportVersion: 2.13.3-0ubuntu1
Architecture: amd64
CasperVersion: 1.339
CurrentDesktop: XFCE
Date: Mon Mar 31 15:05:57 2014
ExecutablePath: /usr/bin/xfce4-settings-manager
LiveMediaBuild: Xubuntu 14.04 LTS "Trusty Tahr" - Beta amd64 (20140326)
ProcEnviron:
 PATH=(custom, no user)
 XDG_RUNTIME_DIR=<set>
 LANG=en_US.UTF-8
 SHELL=/bin/bash
SourcePackage: xfce4-settings
UpgradeStatus: No upgrade log present (probably fresh install)

[Alistair Buxton (a-j-buxton)]

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).

[Alistair Buxton (a-j-buxton)]

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.

[Alistair Buxton (a-j-buxton)]

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.

[Lionel Le Folgoc (mrpouit)]

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.

[Launchpad Janitor (janitor)]

Status changed to 'Confirmed' because the bug affects multiple users.

[Sean Davis (bluesabre)]

I've fixed this bug here:
https://github.com/bluesabre/xfce4-settings/commit/1c828cc780b6742e0e19a002392f6f6ae8a337ee

And cleaned up here:
https://github.com/bluesabre/xfce4-settings/commit/3202b8d93704d69f98bd455f1e095f635dd67d5a

And merged here:
http://git.xfce.org/xfce/xfce4-settings/commit/?id=3903d0ab91b3840eccf9c52f4000b4808c13aec6

[Simon Steinbeiß (ochosi)]

This bug has been fixed in xfce4-settings 4.11.3.

[Thaddaeus Tintenfisch (thad-fisch-deactivatedaccount)]

This bug should be fixed in Xubuntu 14.10 (xfce4-settings 4.11.3-0ubuntu2).

Please test it and report back.

[Roman (thezub)]

Will this be sent back to the 14.04 release? It should still be compatible with the Xfce version, and is a major bugfix.