[Kde-hardware-devel] Review Request 119559: Brightness fixes, part 1: make all layers aware of the actual brightness value and the maximum brightness value.

[Nikita Skovoroda]

> On Авг. 3, 2014, 12:12 д.п., Àlex Fiestas wrote:
> > daemon/backends/upower/xrandrbrightness.cpp, lines 123-151
> > <https://git.reviewboard.kde.org/r/119559/diff/4/?file=294805#file294805line123>
> >
> >     You should use xcb, and please move somewhere else the code that "fetches" the info and the code that has app logic.

I moved repeated code to a separate function, but I am not going to rewrite that code to xcb now.
I did not add it, it was there before.


- Nikita


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/119559/#review63687
-----------------------------------------------------------


On Авг. 3, 2014, 1:08 д.п., Nikita Skovoroda wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/119559/
> -----------------------------------------------------------
> 
> (Updated Авг. 3, 2014, 1:08 д.п.)
> 
> 
> Review request for Solid.
> 
> 
> Repository: powerdevil
> 
> 
> Description
> -------
> 
> Brightness fixes, part 1: 
> Make all layers aware of the actual brightness value and the maximum brightness value.
> 
> Introduce brightnessValue, brightnessValueMax, brightnessValueChanged, setBrightnessValue D-Bus endpoints in org.kde.Solid.PowerManagement.Actions.BrightnessControl.
> Introduce keyboardBrightnessValue, keyboardBrightnessValueMax, keyboardBrightnessValueChanged, setKeyboardBrightnessValue D-Bus endpoints in org.kde.Solid.PowerManagement.Actions.KeyboardBrightnessControl.
> 
> Does not break backwards compatibility, even for the backlighthelper.
> 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.
> 
> 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).
> 
> 
> This patch is the first part of proposed brightness logic rework and makes the basic required changes in all layers.
> 
> What's the problem?
> 
> 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.
> 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.
> 
> The plasmoid steps are fixed at 10 (even if I have 3 actual keyboard steps).
> 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.
> 
> For example, let's suppose we have 15 brighthess steps (that happens sometimes).
> When going upward from 0, that would be 8 steps: 0%, 13%, 27%, 40%, 53%, 67%, 80%, 93%, 100%.
> When going downdard from 100%, that would also be 8 steps: 100%, 87%, 73%, 60%, 47%, 33%, 20%, 7%, 0%.
> Notice how they don't correspond to the upward steps.
> In the plasmoid, that would be 10 steps: 0%, 13%, 20%, 33%, 40%, 53%, 60%, 73%, 80%, 93%, 100%.
> Does it correspond to either of them?
> 
> 
> 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.
> 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.
> 
> The second patch will introduce fixed brightness steps and export them as D-Bus endpoints.
> 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).
> 
> 
> Diffs
> -----
> 
>   daemon/actions/bundled/brightnesscontrol.h 50e15d2 
>   daemon/actions/bundled/brightnesscontrol.cpp 787f256 
>   daemon/actions/bundled/keyboardbrightnesscontrol.h b63fa81 
>   daemon/actions/bundled/keyboardbrightnesscontrol.cpp 558a631 
>   daemon/actions/bundled/org.kde.Solid.PowerManagement.Actions.BrightnessControl.xml 9c4d3df 
>   daemon/actions/bundled/org.kde.Solid.PowerManagement.Actions.KeyboardBrightnessControl.xml 08b5407 
>   daemon/backends/hal/powerdevilhalbackend.h 294a6cf 
>   daemon/backends/hal/powerdevilhalbackend.cpp fc2d476 
>   daemon/backends/upower/backlight_helper_actions.actions 5cbf308 
>   daemon/backends/upower/backlighthelper.h 5ab9298 
>   daemon/backends/upower/backlighthelper.cpp 1c4d933 
>   daemon/backends/upower/powerdevilupowerbackend.h 46817bc 
>   daemon/backends/upower/powerdevilupowerbackend.cpp ea1741f 
>   daemon/backends/upower/xrandrbrightness.h e1f016c 
>   daemon/backends/upower/xrandrbrightness.cpp 27a6792 
>   daemon/powerdevilbackendinterface.h f7634d0 
>   daemon/powerdevilbackendinterface.cpp 40b0925 
> 
> Diff: https://git.reviewboard.kde.org/r/119559/diff/
> 
> 
> Testing
> -------
> 
> Works for me.
> Only the upower backend was tested.
> 
> 
> Thanks,
> 
> Nikita Skovoroda
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-hardware-devel/attachments/20140803/4088dbac/attachment-0001.html>