<table><tr><td style="">davidedmundson added inline comments.
</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/D5928" 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/D5928#inline-24497" rel="noreferrer">View Inline</a><span style="color: #4b4d51; font-weight: bold;">nightcolor.cpp:52</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 class="n">Manager</span><span style="color: #aa2211">::</span><span class="n">Manager</span><span class="p">(</span><span class="n">Platform</span> <span style="color: #aa2211">*</span><span class="n">platform</span><span class="p">)</span>
</div><div style="padding: 0 8px; margin: 0 4px; background: rgba(151, 234, 151, .6);"> <span style="color: #aa2211">:</span> <span class="n">QObject</span><span class="p">(</span><span class="n">platform</span><span class="p">),</span>
</div></div></div>
<div style="margin: 8px 0; padding: 0 12px;"><p style="padding: 0; margin: 8px;">rename the class or the file</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/D5928#inline-24495" rel="noreferrer">View Inline</a><span style="color: #4b4d51; font-weight: bold;">nightcolor.cpp:111</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">if</span> <span class="p">(</span><span class="n">reply</span><span class="p">.</span><span class="n">isValid</span><span class="p">())</span> <span class="p">{</span>
</div><div style="padding: 0 8px; margin: 0 4px; background: rgba(151, 234, 151, .6);"> <span class="n">comingFromSuspend</span> <span style="color: #aa2211">=</span> <span class="n">reply</span><span class="p">.</span><span class="n">value</span><span class="p">().</span><span class="n">toBool</span><span class="p">();</span>
</div><div style="padding: 0 8px; margin: 0 4px; background: rgba(151, 234, 151, .6);"> <span class="p">}</span> <span style="color: #aa4000">else</span> <span class="p">{</span>
</div></div></div>
<div style="margin: 8px 0; padding: 0 12px;"><p style="padding: 0; margin: 8px;">I don't understand.</p>
<p style="padding: 0; margin: 8px;">we're coming from suspend if PreparingForSleep is true?</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/D5928#inline-24498" rel="noreferrer">View Inline</a><span style="color: #4b4d51; font-weight: bold;">nightcolor.h:109</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 class="n">Platform</span> <span style="color: #aa2211">*</span><span class="n">m_platform</span><span class="p">;</span>
</div><div style="padding: 0 8px; margin: 0 4px; background: rgba(151, 234, 151, .6);"> <span class="n">ColorCorrectDBusInterface</span> <span style="color: #aa2211">*</span><span class="n">m_iface</span><span class="p">;</span>
</div></div></div>
<div style="margin: 8px 0; padding: 0 12px;"><p style="padding: 0; margin: 8px;">use kwinApp()->platform() when you need it and remove it from the ctor</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/D5928#inline-24493" rel="noreferrer">View Inline</a><span style="color: #4b4d51; font-weight: bold;">org.kde.kwin.ColorCorrect.xml:17</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: #00702a"></method></span>
</div><div style="padding: 0 8px; margin: 0 4px; background: rgba(151, 234, 151, .6);"> <span style="color: #00702a"><signal</span> <span style="color: #354bb3">name=</span><span style="color: #766510">"nightColorConfigChangeSignal"</span><span style="color: #00702a">></span>
</div><div style="padding: 0 8px; margin: 0 4px; background: rgba(151, 234, 151, .6);"> <span style="color: #00702a"><annotation</span> <span style="color: #354bb3">name=</span><span style="color: #766510">"org.qtproject.QtDBus.QtTypeName.Out0"</span> <span style="color: #354bb3">value=</span><span style="color: #766510">"QHash<QString,QVariant>"</span><span style="color: #00702a">/></span>
</div></div></div>
<div style="margin: 8px 0; padding: 0 12px;"><p style="padding: 0; margin: 8px;">don't include signal in a signal name.</p>
<p style="padding: 0; margin: 8px;">I assume you're doing it to avoid the conflict with the method?<br />
(fun fact, that's valid DBus, just invalid QtDBus)</p>
<p style="padding: 0; margin: 8px;">typical convention is to have the set method called "setNightModeConfig"</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/D5928#inline-24491" rel="noreferrer">View Inline</a><span style="color: #4b4d51; font-weight: bold;">platform.h:382</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: rgba(151, 234, 151, .6);"> <span style="color: #aa4000">void</span> <span style="color: #004012">setSupportsNightColor</span><span class="p">(</span><span style="color: #aa4000">bool</span> <span class="n">set</span><span class="p">)</span> <span class="p">{</span>
</div><div style="padding: 0 8px; margin: 0 4px; background: rgba(151, 234, 151, .6);"> <span class="n">m_supportsNightColor</span> <span style="color: #aa2211">=</span> <span class="n">set</span><span class="p">;</span>
</div></div></div>
<div style="margin: 8px 0; padding: 0 12px;"><p style="padding: 0; margin: 8px;">ok, if this is staying in KWin I still want it layered better. Otherwise you're going to find it hard to extend it.</p>
<p style="padding: 0; margin: 8px;">Nothing in Platform and subclasses should mention nightmode. <br />
They should all refer to supportsGammaCorrection or something.</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/D5928#inline-24496" rel="noreferrer">View Inline</a><span style="color: #4b4d51; font-weight: bold;">drm_object_crtc.cpp:115</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 class="p">}</span>
</div><div style="padding: 0 8px; margin: 0 4px; background: rgba(151, 234, 151, .6);"> <span style="color: #aa4000">bool</span> <span class="n">isError</span> <span style="color: #aa2211">=</span> <span class="n">drmModeCrtcSetGamma</span><span class="p">(</span><span class="n">m_backend</span><span style="color: #aa2211">-></span><span class="n">fd</span><span class="p">(),</span> <span class="n">m_id</span><span class="p">,</span> <span class="n">m_gammaSize</span><span class="p">,</span>
</div><div style="padding: 0 8px; margin: 0 4px; background: rgba(151, 234, 151, .6);"> <span class="n">gammaRamp</span><span class="p">,</span> <span style="color: #74777d">// red</span>
</div></div></div>
<div style="margin: 8px 0; padding: 0 12px;"><p style="padding: 0; margin: 8px;">so we have 3 arrays of size m_gammaSize</p>
<p style="padding: 0; margin: 8px;">we can pass the GammaRamp object and do:</p>
<p style="padding: 0; margin: 8px;">gammaRamp.red,<br />
gammaRamp.green,<br />
gammaRamp.blue,</p>
<p style="padding: 0; margin: 8px;">also it'll be safer to use the gammaSize from the gammaramp. otherwise if it changes, you're going to crash.</p>
<p style="padding: 0; margin: 8px;">if you are trying for separation pass 3 uint*s instead of doing pointer maths.</p></div></div></div></div></div><br /><div><strong>REPOSITORY</strong><div><div>R108 KWin</div></div></div><br /><div><strong>REVISION DETAIL</strong><div><a href="https://phabricator.kde.org/D5928" rel="noreferrer">https://phabricator.kde.org/D5928</a></div></div><br /><div><strong>To: </strong>subdiff, KWin<br /><strong>Cc: </strong>graesslin, davidedmundson, plasma-devel, kwin, ZrenBot, spstarr, progwolff, lesliezhai, ali-mohamed, hardening, jensreuterberg, abetts, eliasp, sebas, apol, mart, hein, lukas<br /></div>