----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/121798/#review73114 -----------------------------------------------------------
Concerning locking on non-Logind: the code should be removed from powerdevil (I think I have a review request open for that). Nevertheless I would like to still have the feature available in screen locker. daemon/kwineffect.h <https://git.reviewboard.kde.org/r/121798/#comment50844> I don't like the name: it's too generic. What's *the* KWinEffect daemon/kwineffect.cpp <https://git.reviewboard.kde.org/r/121798/#comment50847> I think type should be XCB_ATOM_CARDINAL and lenght only needs to be 1 daemon/kwineffect.cpp <https://git.reviewboard.kde.org/r/121798/#comment50848> I'd suggest to also verify that format is 32 daemon/kwineffect.cpp <https://git.reviewboard.kde.org/r/121798/#comment50846> the type xcb_atom_t looks wrong to me. It should just be a uint32_t IIRC - Martin Gräßlin On Jan. 3, 2015, 1:14 p.m., Kai Uwe Broulik wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://git.reviewboard.kde.org/r/121798/ > ----------------------------------------------------------- > > (Updated Jan. 3, 2015, 1:14 p.m.) > > > Review request for Solid, Àlex Fiestas and Martin Gräßlin. > > > Bugs: 340063 > https://bugs.kde.org/show_bug.cgi?id=340063 > > > Repository: powerdevil > > > Description > ------- > > This is an implementation of the client side of the KScreen effect for > PowerDevil eventually allowing it to fade the screen before going to sleep. > The code is loosely based on Alex' initial work [1] but ported to XCB. > > [1] > http://quickgit.kde.org/?p=libkscreen.git&a=commit&h=70cf4482ed2e83d14c2ec9b805921ba4eb741082 > > > Diffs > ----- > > daemon/CMakeLists.txt f26f786 > daemon/config-powerdevil.h.cmake b730cd9 > daemon/kwineffect.h PRE-CREATION > daemon/kwineffect.cpp PRE-CREATION > > Diff: https://git.reviewboard.kde.org/r/121798/diff/ > > > Testing > ------- > > Calling start() causes the scren to dim, the state is properly updated by > KWin and the respective signals are fired. > > As for the implementation in the suspendsession action I have to agree with > Martin here that we should drop the support for locking on non-logind > platforms since it makes the code utterly complex and isn't secure anyway. > That way PowerDevil could just kick off the fade effect, wait for it to be > done and then tell Logind to suspend which completely takes care of the > locking (and more importantly the waiting for the lock to be established > *before* powering down). > > > Thanks, > > Kai Uwe Broulik > >
_______________________________________________ Kde-hardware-devel mailing list Kde-hardware-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-hardware-devel
