<table><tr><td style="">davidedmundson requested changes to this revision.<br />davidedmundson added inline comments.<br />This revision now requires changes to proceed.
</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/D8493" rel="noreferrer">View Revision</a></tr></table><br /><div><strong>INLINE COMMENTS</strong><div><div style="margin: 6px 0 12px 0;"><div style="border: 1px solid #C7CCD9; border-radius: 3px;"><div style="padding: 0; background: #F7F7F7; border-color: #e3e4e8; border-style: solid; border-width: 0 0 1px 0; margin: 0;"><div style="color: #74777d; background: #eff2f4; padding: 6px 8px; overflow: hidden;"><a style="float: right; text-decoration: none;" href="https://phabricator.kde.org/D8493#inline-37288" rel="noreferrer">View Inline</a><span style="color: #4b4d51; font-weight: bold;">screenmapper.cpp:65</span></div>
<div style="font: 11px/15px "Menlo", "Consolas", "Monaco", monospace; white-space: pre-wrap; clear: both; padding: 4px 0; margin: 0;"><div style="padding: 0 8px; margin: 0 4px; background: rgba(151, 234, 151, .6);"><span style="color: #aa4000">void</span> <span class="n">ScreenMapper</span><span style="color: #aa2211">::</span><span class="n">handleScreenAdded</span><span class="p">(</span><span class="n">QScreen</span> <span style="color: #aa2211">*</span><span class="n">screen</span><span class="p">)</span>
</div><div style="padding: 0 8px; margin: 0 4px; background: rgba(151, 234, 151, .6);"><span class="p">{</span>
</div></div></div>
<div style="margin: 8px 0; padding: 0 12px;"><p style="padding: 0; margin: 8px;">I don't think you want to track QScreens.</p>

<p style="padding: 0; margin: 8px;">We need to handle the case of overlapping (cloned) screens.</p>

<p style="padding: 0; margin: 8px;">You'll have two QScreen objects, two screen pool IDs, but only one desktop view. (and thus only one folder view).</p>

<p style="padding: 0; margin: 8px;">If we were previously spanned, and then go into clone, we need Folderview to act like there's only one screen attached. I don't think this will.</p>

<p style="padding: 0; margin: 8px;">We need the same logic here, that ShellCorona has for creating DesktopViews. This either means we do it like you're doing, then copy and paste ShellCorona::isOutputRedundant here.</p>

<p style="padding: 0; margin: 8px;">or the much neater solution for your problem, would be to move the overlap detection from ShellCorona to ScreenPool, then use that as the canonical source of screens attached which you can then use here with it all in sync.</p>

<p style="padding: 0; margin: 8px;">(I sort of started that in the p-w branch davidedmundson/screenpool_changes)</p></div></div></div></div></div><br /><div><strong>REPOSITORY</strong><div><div>R119 Plasma Desktop</div></div></div><br /><div><strong>REVISION DETAIL</strong><div><a href="https://phabricator.kde.org/D8493" rel="noreferrer">https://phabricator.kde.org/D8493</a></div></div><br /><div><strong>To: </strong>amantia, Plasma, ervin, mlaurent, dvratil, hein, aacid, davidedmundson<br /><strong>Cc: </strong>davidedmundson, broulik, mart, plasma-devel, ZrenBot, progwolff, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol<br /></div>