<table><tr><td style="">sebas added a comment.</td><a style="text-decoration: none; padding: 4px 8px; margin: 0 8px 8px; float: right; color: #464C5C; font-weight: bold; border-radius: 3px; background-color: #F7F7F9; background-image: linear-gradient(to bottom,#fff,#f1f0f1); display: inline-block; border: 1px solid rgba(71,87,120,.2);" href="https://phabricator.kde.org/D2156" rel="noreferrer">View Revision</a></tr></table><br /><div><div><p><a href="https://phabricator.kde.org/p/davidedmundson/" style="
              border-color: #f1f7ff;
              color: #19558d;
              background-color: #f1f7ff;
                border: 1px solid transparent;
                border-radius: 3px;
                font-weight: bold;
                padding: 0 4px;" rel="noreferrer">@davidedmundson</a> Sure.</p>

<p>With this patch</p>

<ul class="remarkup-list">
<li class="remarkup-list-item">we detect the case when multiple identical outputs are connected (same hash)</li>
<li class="remarkup-list-item">each of the duplicate outputs also gets matched against the output name</li>
</ul>

<p>The problem I'm fixing is that currently (without my patch) the ordering of outputs matters, so if an output appears later on, it ends up in a different place in the list. This may confuse the current algorithm which relies on the ordering of config->connectedOutputs() since it'll match the first and remove that -- there's no guarantee that the first match is the right one, however. It works in the most likely case, when all outputs are added the same order as in the config file, but breaks once I mix up this ordering.</p>

<p>The config file also holds the output's name, and since that's usually the connector's name and likely to be unique, it's a good indicator for identifying an output.</p>

<p>If you look at the config file in <a href="https://bugs.kde.org/show_bug.cgi?id=325277" class="remarkup-link" target="_blank" rel="noreferrer">https://bugs.kde.org/show_bug.cgi?id=325277</a> (grep for e7d18f), this becomes perhaps a bit clearer.</p>

<p>In the second iteration of this patch, I've tightened up the continue condition a bit (and fixed it, I used the wrong json object), so it's less likely to skip outputs when the name is empty.</p>

<p>I've added an autotest which sets up a 3x2 video wall and loads the config from the bugreport. This test fails without the change in findOutputs() and passes with it. I've played a bit around with it, and if I add the output programmatically in the same order as the config file has them (alphanumeric) it works without my patch, changing the ordering makes the tests fail without, and pass with my patch.</p>

<p>tl,dr; I think your patch makes it work in the most likely cases, but may not be enough when displays are being added in a different order.</p></div></div><br /><div><strong>REPOSITORY</strong><div><div>rKSCREEN KScreen</div></div></div><br /><div><strong>REVISION DETAIL</strong><div><a href="https://phabricator.kde.org/D2156" rel="noreferrer">https://phabricator.kde.org/D2156</a></div></div><br /><div><strong>EMAIL PREFERENCES</strong><div><a href="https://phabricator.kde.org/settings/panel/emailpreferences/" rel="noreferrer">https://phabricator.kde.org/settings/panel/emailpreferences/</a></div></div><br /><div><strong>To: </strong>sebas, Plasma<br /><strong>Cc: </strong>davidedmundson, graesslin, lbeltrame, plasma-devel, jensreuterberg, abetts, sebas<br /></div>