> On Авг. 1, 2014, 8:14 д.п., Kai Uwe Broulik wrote: > > Thank you! I've always wanted to make powerdevil use the actual maximum > > brightness instead of just exporting fixed 10% steps. > > > > Given that this is powerdevil (and not a part of kde-frameworks), I don't > > think we need to make that backwards compatible, ie. brightness can be the > > new absolute value, with a maximumBrightness being the maximum. We just > > need to make sure to adjust all the users of that: Plasma PM dataengine, > > Kamoso, … however we're already past the 5.0 release and breaking things > > for others is nasty. > > > > Frankly, I currently do not know what the state of Solid Power is, and > > whether it will handle the brightness stuff (and if it already uses the > > absolute values) and whether it will be removed from powerdevil then. > > Nikita Skovoroda wrote: > Actually, there is exactly no reason to break D-Bus api compatibility. > I was asking about breaking backwards compatibility for inner helpers. I > assume that they shouldn't be used from outside? > > If you confirm that, I'll clean this up a bit. > > Nikita Skovoroda wrote: > backlighthelper is a standalone binary, it had «brightness», > «setbrightness» and «syspath» actions. > I added «brightnessvalue», «brightnessvaluemax», «serbrightnessvalue» > actions. > > If the old «brightness», «setbrightness» actions could be removed (from > the helper executable), that would clean up things a bit. > > D-Bus api is still backwards compatible, I just introduced new methods.
I finished the second part that introduces the brightnessStep api and uses that for increase/decrease brightness actions, making them predictable. I can either add it to this RR (making it rather big and less readable) or wait untill this is merged. It looks like I can't post a separate RR that depends on this one without this one being merged. - Nikita ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/119559/#review63610 ----------------------------------------------------------- On Авг. 1, 2014, 9:18 д.п., Nikita Skovoroda wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://git.reviewboard.kde.org/r/119559/ > ----------------------------------------------------------- > > (Updated Авг. 1, 2014, 9:18 д.п.) > > > 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/powerdevilbackendinterface.h f7634d0 > daemon/powerdevilbackendinterface.cpp 40b0925 > daemon/backends/upower/xrandrbrightness.h e1f016c > daemon/backends/upower/xrandrbrightness.cpp 27a6792 > 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 > > Diff: https://git.reviewboard.kde.org/r/119559/diff/ > > > Testing > ------- > > Works for me. > Only the upower backend was tested. > > > Thanks, > > Nikita Skovoroda > >
_______________________________________________ Kde-hardware-devel mailing list Kde-hardware-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-hardware-devel