[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]

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

(Updated Авг. 1, 2014, 8:37 д.п.)


Review request for Solid.


Changes
-------

Minor change: set descriptions.


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 (updated)
-----

  daemon/backends/upower/xrandrbrightness.cpp 27a6792 
  daemon/powerdevilbackendinterface.h f7634d0 
  daemon/powerdevilbackendinterface.cpp 40b0925 
  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/hal/powerdevilhalbackend.cpp fc2d476 
  daemon/backends/upower/backlight_helper_actions.actions 5cbf308 
  daemon/backends/upower/backlighthelper.h 5ab9298 
  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/actions/bundled/brightnesscontrol.h 50e15d2 
  daemon/actions/bundled/brightnesscontrol.cpp 787f256 
  daemon/actions/bundled/keyboardbrightnesscontrol.h b63fa81 
  daemon/actions/bundled/keyboardbrightnesscontrol.cpp 558a631 

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/20140801/8469f0f1/attachment.html>