<table><tr><td style="">dvratil 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/D2330" rel="noreferrer">View Revision</a></tr></table><br /><div><div><p>I wonder how it happens that we try to enable an output without having the mode set. The KDED module should always pick the best resolution (or falling back to preferred mode) when it's enabling an output.</p>
<p>While this fix makes sense in as a crash-guard, I'd also look into why KDED is failing to set the mode in the first place.</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/D2330#inline-9334" rel="noreferrer">View Inline</a><span style="color: #4b4d51; font-weight: bold;">xrandrconfig.cpp:519</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: #d0ffd0;"> <span style="color: #aa4000">if</span> <span class="p">(</span><span style="color: #aa2211">!</span><span class="n">kscreenOutput</span><span style="color: #aa2211">-></span><span class="n">currentMode</span><span class="p">())</span> <span class="p">{</span>
</div><div style="padding: 0 8px; margin: 0 4px; background: #d0ffd0;"> <span class="n">modeId</span> <span style="color: #aa2211">=</span> <span class="n">kscreenOutput</span><span style="color: #aa2211">-></span><span class="n">preferredModeId</span><span class="p">().</span><span class="n">toInt</span><span class="p">();</span>
</div><div style="padding: 0 8px; margin: 0 4px; background: #d0ffd0;"> <span class="p">}</span>
</div></div></div>
<div style="margin: 8px 0; padding: 0 12px;"><p style="padding: 0; margin: 8px;">I wonder if it might be safer to query the preferred mode from corresponding <tt style="background: #ebebeb; font-size: 13px;">XRandROutput</tt> rather than trusting user-provided <tt style="background: #ebebeb; font-size: 13px;">KScreen::Output</tt> here?</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/D2330#inline-9333" rel="noreferrer">View Inline</a><span style="color: #4b4d51; font-weight: bold;">xrandrconfig.cpp:561</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: #d0ffd0;"> <span style="color: #aa4000">if</span> <span class="p">(</span><span style="color: #aa2211">!</span><span class="n">kscreenOutput</span><span style="color: #aa2211">-></span><span class="n">currentMode</span><span class="p">())</span> <span class="p">{</span>
</div><div style="padding: 0 8px; margin: 0 4px; background: #d0ffd0;"> <span class="n">modeId</span> <span style="color: #aa2211">=</span> <span class="n">kscreenOutput</span><span style="color: #aa2211">-></span><span class="n">preferredModeId</span><span class="p">().</span><span class="n">toInt</span><span class="p">();</span>
</div><div style="padding: 0 8px; margin: 0 4px; background: #d0ffd0;"> <span class="p">}</span>
</div></div></div>
<div style="margin: 8px 0; padding: 0 12px;"><p style="padding: 0; margin: 8px;">In <tt style="background: #ebebeb; font-size: 13px;">changeOutput()</tt> we only change position, mode or rotation of an already-enabled output. I would argue that if the mode is missing in the <tt style="background: #ebebeb; font-size: 13px;">kscreenOutput</tt> here (for any reason) it would make more sense to interpret it as "no change" and use <tt style="background: #ebebeb; font-size: 13px;">xOutput->currentMode()->id()</tt> and only if <tt style="background: #ebebeb; font-size: 13px;">xOutput->currentMode()</tt> is null then fall back to preferred mode (although that already hints that something weird is going on).</p></div></div></div></div></div><br /><div><strong>REPOSITORY</strong><div><div>rLIBKSCREEN KScreen Library</div></div></div><br /><div><strong>REVISION DETAIL</strong><div><a href="https://phabricator.kde.org/D2330" rel="noreferrer">https://phabricator.kde.org/D2330</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, dvratil<br /><strong>Cc: </strong>graesslin, plasma-devel, Plasma, ali-mohamed, jensreuterberg, abetts, sebas<br /></div>