<html>
<body>
<div style="font-family: Verdana, Arial, Helvetica, Sans-Serif;">
<table bgcolor="#f9f3c9" width="100%" cellpadding="12" style="border: 1px #c9c399 solid; border-radius: 6px; -moz-border-radius: 6px; -webkit-border-radius: 6px;">
<tr>
<td>
This is an automatically generated e-mail. To reply, visit:
<a href="https://git.reviewboard.kde.org/r/119559/">https://git.reviewboard.kde.org/r/119559/</a>
</td>
</tr>
</table>
<br />
<table bgcolor="#fefadf" width="100%" cellspacing="0" cellpadding="12" style="border: 1px #888a85 solid; border-radius: 6px; -moz-border-radius: 6px; -webkit-border-radius: 6px;">
<tr>
<td>
<div>Review request for Solid.</div>
<div>By Nikita Skovoroda.</div>
<p style="color: grey;"><i>Updated Авг. 3, 2014, 1:08 д.п.</i></p>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Changes</h1>
<table width="100%" bgcolor="#ffffff" cellspacing="0" cellpadding="10" style="border: 1px solid #b8b5a0">
<tr>
<td>
<pre style="margin: 0; padding: 0; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">Removed two simular compiler warnings.</pre>
</td>
</tr>
</table>
<div style="margin-top: 1.5em;">
<b style="color: #575012; font-size: 10pt;">Repository: </b>
powerdevil
</div>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Description </h1>
<table width="100%" bgcolor="#ffffff" cellspacing="0" cellpadding="10" style="border: 1px solid #b8b5a0">
<tr>
<td>
<pre style="margin: 0; padding: 0; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">Brightness fixes, part 1: <br style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: normal;" />
Make all layers aware of the actual brightness value and the maximum brightness value.</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">Introduce brightnessValue, brightnessValueMax, brightnessValueChanged, setBrightnessValue D-Bus endpoints in org.kde.Solid.PowerManagement.Actions.BrightnessControl.<br style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: normal;" />
Introduce keyboardBrightnessValue, keyboardBrightnessValueMax, keyboardBrightnessValueChanged, setKeyboardBrightnessValue D-Bus endpoints in org.kde.Solid.PowerManagement.Actions.KeyboardBrightnessControl.</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">Does not break backwards compatibility, even for the backlighthelper.<br style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: normal;" />
This could be cleaned up a bit, if it's allowed to break backwards compatibility for inner helpers: the old float-based logic can be removed from the helpers completely and re-implemented on top of int-ish logic in the backends.</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">The behaviour of cached brightness values is not optimal now, but this will be addressed in the second part of the fixes (see description downwards).</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">This patch is the first part of proposed brightness logic rework and makes the basic required changes in all layers.</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">What's the problem?</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">Atm, the brightness logic is a bit messy: it doesn't work nice if the count of underlying brightness steps is not equal to 10.<br style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: normal;" />
Brightness steps when going upward do not correspond to brightness steps when going downward, neither of them corresponds to brightness steps in the plasmoid, and neither of them corresponds to the actual brightness steps.</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">The plasmoid steps are fixed at 10 (even if I have 3 actual keyboard steps).<br style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: normal;" />
The upwards/downwards logic tries to add/subtract 10% (or 30%, depending on the steps count) from the current value and just looks at what happens.</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">For example, let's suppose we have 15 brighthess steps (that happens sometimes).<br style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: normal;" />
When going upward from 0, that would be 8 steps: 0%, 13%, 27%, 40%, 53%, 67%, 80%, 93%, 100%.<br style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: normal;" />
When going downdard from 100%, that would also be 8 steps: 100%, 87%, 73%, 60%, 47%, 33%, 20%, 7%, 0%.<br style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: normal;" />
Notice how they don't correspond to the upward steps.<br style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: normal;" />
In the plasmoid, that would be 10 steps: 0%, 13%, 20%, 33%, 40%, 53%, 60%, 73%, 80%, 93%, 100%.<br style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: normal;" />
Does it correspond to either of them?</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">The proposed solution is to introduce fixed (you can think of them as pre-calculated, but they are not) brightness steps in PowerDevil and stick to those in upwards/downwards logic.<br style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: normal;" />
The plasmoid could use those steps (slider from 0 to brightnessStepMax) or the actual underlying brightness value (slider from 0 to brightnessValueMax) — that would be in the third patch.</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">The second patch will introduce fixed brightness steps and export them as D-Bus endpoints.<br style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: normal;" />
In total, there would be three types of D-Bus endpoints that could be used by apps (plasmoids etc.): brightness (percentage), brightnessValue (raw value), brightnessStep (pre-calculated brightness steps).</p></pre>
</td>
</tr>
</table>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Testing </h1>
<table width="100%" bgcolor="#ffffff" cellspacing="0" cellpadding="10" style="border: 1px solid #b8b5a0">
<tr>
<td>
<pre style="margin: 0; padding: 0; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">Works for me.<br style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: normal;" />
Only the upower backend was tested.</p></pre>
</td>
</tr>
</table>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Diffs</b> (updated)</h1>
<ul style="margin-left: 3em; padding-left: 0;">
<li>daemon/actions/bundled/brightnesscontrol.h <span style="color: grey">(50e15d2)</span></li>
<li>daemon/actions/bundled/brightnesscontrol.cpp <span style="color: grey">(787f256)</span></li>
<li>daemon/actions/bundled/keyboardbrightnesscontrol.h <span style="color: grey">(b63fa81)</span></li>
<li>daemon/actions/bundled/keyboardbrightnesscontrol.cpp <span style="color: grey">(558a631)</span></li>
<li>daemon/actions/bundled/org.kde.Solid.PowerManagement.Actions.BrightnessControl.xml <span style="color: grey">(9c4d3df)</span></li>
<li>daemon/actions/bundled/org.kde.Solid.PowerManagement.Actions.KeyboardBrightnessControl.xml <span style="color: grey">(08b5407)</span></li>
<li>daemon/backends/hal/powerdevilhalbackend.h <span style="color: grey">(294a6cf)</span></li>
<li>daemon/backends/hal/powerdevilhalbackend.cpp <span style="color: grey">(fc2d476)</span></li>
<li>daemon/backends/upower/backlight_helper_actions.actions <span style="color: grey">(5cbf308)</span></li>
<li>daemon/backends/upower/backlighthelper.h <span style="color: grey">(5ab9298)</span></li>
<li>daemon/backends/upower/backlighthelper.cpp <span style="color: grey">(1c4d933)</span></li>
<li>daemon/backends/upower/powerdevilupowerbackend.h <span style="color: grey">(46817bc)</span></li>
<li>daemon/backends/upower/powerdevilupowerbackend.cpp <span style="color: grey">(ea1741f)</span></li>
<li>daemon/backends/upower/xrandrbrightness.h <span style="color: grey">(e1f016c)</span></li>
<li>daemon/backends/upower/xrandrbrightness.cpp <span style="color: grey">(27a6792)</span></li>
<li>daemon/powerdevilbackendinterface.h <span style="color: grey">(f7634d0)</span></li>
<li>daemon/powerdevilbackendinterface.cpp <span style="color: grey">(40b0925)</span></li>
</ul>
<p><a href="https://git.reviewboard.kde.org/r/119559/diff/" style="margin-left: 3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>