<table><tr><td style="">davidedmundson 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/D2218" rel="noreferrer">View Revision</a></tr></table><br /><div><div><blockquote style="border-left: 3px solid #a7b5bf; color: #464c5c; font-style: italic; margin: 4px 0 12px 0; padding: 4px 12px; background-color: #f8f9fc;"><p>i may be able to kill idsfordesktopview tough, will update the rr</p></blockquote>
<p>Yep, that's exactly what I meant. ++<br />
Also means panels can use the API directly. (see comment #3)</p>
<blockquote style="border-left: 3px solid #a7b5bf; color: #464c5c; font-style: italic; margin: 4px 0 12px 0; padding: 4px 12px; background-color: #f8f9fc;"><p>perhaps would be more clear if the logic for that is completely moved in screenpool?</p></blockquote>
<p>There should definitely be some consistency. Either screenpool is in charge of managing views or shellcorona is.</p>
<p>Either woudl be valid but currently this patch current inserts and removals of DesktopView to the right screen are all handled by ShellCorona setting the screen, but primary changes of DesktopView are done by ScreenPool; and Panels are mixed up too.</p>
<p>Personally, I think the part that's out of place is ScreenPool touching the desktop views; if you move m_desktopViewforId to shellCorona the design all fits:<br />
insert/remove in ScreenPool become reflective<br />
and all DesktopView stuff is within one class, screen to ID mapping is in one class.</p></div></div><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/D2218#inline-8937" rel="noreferrer">View Inline</a><span style="color: #4b4d51; font-weight: bold;">panelview.cpp:647</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; "><span class="p">{</span>
</div><div style="padding: 0 8px; margin: 0 4px; background: #d0ffd0;"> <span style="color: #74777d">//it's important the panel is positioned immediately in order to not have the</span>
</div><div style="padding: 0 8px; margin: 0 4px; background: #d0ffd0;"> <span style="color: #74777d">//qscreen changed under his feet</span>
</div></div></div>
<div style="margin: 8px 0; padding: 0 12px;"><p style="padding: 0; margin: 8px;">well, this is now completely not true as we're following screenToFollow</p></div></div><br /><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/D2218#inline-8935" rel="noreferrer">View Inline</a><span style="color: #4b4d51; font-weight: bold;">shellcorona.cpp:1421</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; "> <span style="color: #aa4000">if</span> <span class="p">(</span><span class="n">view</span><span class="p">)</span> <span class="p">{</span>
</div><div style="padding: 0 8px; margin: 0 4px; "> <span class="n">QScreen</span> <span style="color: #aa2211">*</span><span class="n">screen</span> <span style="color: #aa2211">=</span> <span class="n">view</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: #ffd0d0;"> <span style="color: #aa4000">for</span> <span class="p">(</span><span style="color: #aa4000">int</span> <span class="n">i<span class="bright"></span></span><span class="bright"> </span><span style="color: #aa2211"><span class="bright">=</span></span><span class="bright"> </span><span style="color: #601200"><span class="bright">0</span></span><span class="bright"></span><span class="p"><span class="bright">;</span></span><span class="bright"> </span><span class="n"><span class="bright">i</span></span><span class="bright"> </span><span style="color: #aa2211"><span class="bright"><</span></span><span class="bright"> </span><span class="n"><span class="bright">m_views</span></span><span class="bright"></span><span class="p"><span class="bright">.</span></span><span class="bright"></span><span class="n"><span class="bright">count</span></span><span class="bright"></span><span class="p"><span class="bright">();</span></span><span class="bright"> </span><span class="n"><span class="bright">i</span></span><span class="bright"></span><span style="color: #aa2211"><span class="bright">++</span></span><span class="p">)</span> <span class="p">{</span>
</div><div style="padding: 0 8px; margin: 0 4px; background: #d0ffd0;"> <span class="n">for<span class="bright">each</span></span> <span class="p">(</span><span style="color: #aa4000">int</span> <span class="n">i<span class="bright">d</span></span><span class="bright"></span><span class="p"><span class="bright">,</span></span><span class="bright"> </span><span class="n"><span class="bright">m_screenPool</span></span><span class="bright"></span><span style="color: #aa2211"><span class="bright">-></span></span><span class="bright"></span><span class="n"><span class="bright">ids</span></span><span class="bright"></span><span class="p"><span class="bright">()</span>)</span> <span class="p">{</span>
</div></div></div>
<div style="margin: 8px 0; padding: 0 12px;"><p style="padding: 0; margin: 8px;">surely this should be screenToFollow()?</p></div></div><br /><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/D2218#inline-8936" rel="noreferrer">View Inline</a><span style="color: #4b4d51; font-weight: bold;">shellcorona.cpp:1422</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; "> <span class="n">QScreen</span> <span style="color: #aa2211">*</span><span class="n">screen</span> <span style="color: #aa2211">=</span> <span class="n">view</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: #ffd0d0;"> <span style="color: #aa4000">for</span> <span class="p">(</span><span style="color: #aa4000">int</span> <span class="n">i<span class="bright"></span></span><span class="bright"> </span><span style="color: #aa2211"><span class="bright">=</span></span><span class="bright"> </span><span style="color: #601200"><span class="bright">0</span></span><span class="bright"></span><span class="p"><span class="bright">;</span></span><span class="bright"> </span><span class="n"><span class="bright">i</span></span><span class="bright"> </span><span style="color: #aa2211"><span class="bright"><</span></span><span class="bright"> </span><span class="n"><span class="bright">m_views</span></span><span class="bright"></span><span class="p"><span class="bright">.</span></span><span class="bright"></span><span class="n"><span class="bright">count</span></span><span class="bright"></span><span class="p"><span class="bright">();</span></span><span class="bright"> </span><span class="n"><span class="bright">i</span></span><span class="bright"></span><span style="color: #aa2211"><span class="bright">++</span></span><span class="p">)</span> <span class="p">{</span>
</div><div style="padding: 0 8px; margin: 0 4px; background: #ffd0d0;"> <span style="color: #aa4000">if</span> <span class="p">(</span><span class="n">m_view<span class="bright">s</span></span><span class="bright"></span><span class="p"><span class="bright">[</span></span><span class="bright"></span><span class="n"><span class="bright">i</span></span><span class="bright"></span><span class="p"><span class="bright">]</span></span><span style="color: #aa2211">-></span><span class="n">screen</span><span class="p">()</span> <span style="color: #aa2211">==</span> <span class="n">screen</span><span class="p">)</span> <span class="p">{</span>
</div><div style="padding: 0 8px; margin: 0 4px; background: #d0ffd0;"> <span class="n">for<span class="bright">each</span></span> <span class="p">(</span><span style="color: #aa4000">int</span> <span class="n">i<span class="bright">d</span></span><span class="bright"></span><span class="p"><span class="bright">,</span></span><span class="bright"> </span><span class="n"><span class="bright">m_screenPool</span></span><span class="bright"></span><span style="color: #aa2211"><span class="bright">-></span></span><span class="bright"></span><span class="n"><span class="bright">ids</span></span><span class="bright"></span><span class="p"><span class="bright">()</span>)</span> <span class="p">{</span>
</div><div style="padding: 0 8px; margin: 0 4px; background: #d0ffd0;"> <span style="color: #aa4000">if</span> <span class="p">(</span><span class="n">m_<span class="bright">screenPool</span></span><span class="bright"></span><span style="color: #aa2211"><span class="bright">-></span></span><span class="n">view<span class="bright"></span></span><span class="bright"></span><span class="p"><span class="bright">(</span></span><span class="bright"></span><span class="n"><span class="bright">id</span></span><span class="bright"></span><span class="p"><span class="bright">)</span></span><span style="color: #aa2211">-></span><span class="n">screen</span><span class="p">()</span> <span style="color: #aa2211">==</span> <span class="n">screen</span><span class="p">)</span> <span class="p">{</span>
</div></div></div>
<div style="margin: 8px 0; padding: 0 12px;"><p style="padding: 0; margin: 8px;">this is silly.</p>
<p style="padding: 0; margin: 8px;">We're looping through the desktops to find one with the same screen as us - then looking up the name of that.</p>
<p style="padding: 0; margin: 8px;">We have the screen, we can use m_screenPool->id(screen-name());</p></div></div></div></div></div><br /><div><strong>REPOSITORY</strong><div><div>rPLASMAWORKSPACE Plasma Workspace</div></div></div><br /><div><strong>REVISION DETAIL</strong><div><a href="https://phabricator.kde.org/D2218" rel="noreferrer">https://phabricator.kde.org/D2218</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>mart, Plasma<br /><strong>Cc: </strong>davidedmundson, rwooninck, graesslin, plasma-devel, jensreuterberg, abetts, sebas<br /></div>