> On 2010-05-06 11:59:07, Sebastian Kügler wrote: > > trunk/KDE/kdebase/workspace/plasma/generic/applets/battery/battery.cpp, > > line 131 > > <http://reviewboard.kde.org/r/1942/diff/2/?file=25713#file25713line131> > > > > Any good reason to not use Qt's parenting mechanism to do the dirty > > memory management work?
m_brightnessOSD is a top-level window and thus can't have any parent. Is it possible to set a parent only in the QObject/memory management sense but not as a QWidget/window parent? > On 2010-05-06 11:59:07, Sebastian Kügler wrote: > > trunk/KDE/kdebase/workspace/plasma/generic/applets/battery/brightnessosdwidget.cpp, > > line 45 > > <http://reviewboard.kde.org/r/1942/diff/2/?file=25715#file25715line45> > > > > would be good to properly parent those items, to make clear that memory > > management is taken care of, it takes a bit of digging right now. > > > > I personally prefer 0-ing the pointers there, and then initialize them > > inside the function, as that makes the code more readable You're right the parent relationsship between the objects is a bit obscure. As said I only copied the code from KMix but I'll clean this up. - Felix ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://reviewboard.kde.org/r/1942/#review5450 ----------------------------------------------------------- On 2010-05-05 20:58:51, Felix Geyer wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > http://reviewboard.kde.org/r/1942/ > ----------------------------------------------------------- > > (Updated 2010-05-05 20:58:51) > > > Review request for Plasma and Dario Freddi. > > > Summary > ------- > > This patch addresses the following issues: > - KDE doesn't provide an OSD notification when the user changes the > display brightness. > - The brightness Fn keys don't have an effect if they aren't handled by > the kernel/hardware. > > Implementation overview: > PowerDevil uses a KActionCollection to register the > Qt::Key_MonBrightnessUp/Down keys > and forwards them to Solid. > Solid then decides if the brightness should be increased/decreased by > comparing the > current brightness value with the cached one. The brightness is only changed > if both are equal and the brightness_in_hardware HAL property is false. > This prevents a double increase/decrease of the brightness and is consistent > with > gnome-power-manager using the HAL backend. > In any case PowerDevil emits the brightnessChanged() dbus signal. > The battery applet listens to that signal and displays an OSD. > It uses an adapted copy of the OSDWidget class from KMix. This is suboptimal > but KDE > doesn't provide a generic way to show this kind of OSD yet. > > > This addresses bugs 182400 and 187960. > https://bugs.kde.org/show_bug.cgi?id=182400 > https://bugs.kde.org/show_bug.cgi?id=187960 > > > Diffs > ----- > > trunk/KDE/kdebase/workspace/libs/solid/control/ifaces/powermanager.h > 1122901 > trunk/KDE/kdebase/workspace/libs/solid/control/powermanager.h 1122901 > trunk/KDE/kdebase/workspace/libs/solid/control/powermanager.cpp 1122901 > trunk/KDE/kdebase/workspace/plasma/generic/applets/battery/CMakeLists.txt > 1122901 > trunk/KDE/kdebase/workspace/plasma/generic/applets/battery/battery.h > 1122901 > trunk/KDE/kdebase/workspace/plasma/generic/applets/battery/battery.cpp > 1122901 > > trunk/KDE/kdebase/workspace/plasma/generic/applets/battery/brightnessosdwidget.h > PRE-CREATION > > trunk/KDE/kdebase/workspace/plasma/generic/applets/battery/brightnessosdwidget.cpp > PRE-CREATION > trunk/KDE/kdebase/workspace/powerdevil/daemon/PowerDevilDaemon.h 1122901 > trunk/KDE/kdebase/workspace/powerdevil/daemon/PowerDevilDaemon.cpp 1122901 > trunk/KDE/kdebase/workspace/powerdevil/daemon/org.kde.PowerDevil.xml > 1122901 > trunk/KDE/kdebase/workspace/solid/hal/halpower.h 1122901 > trunk/KDE/kdebase/workspace/solid/hal/halpower.cpp 1122901 > > Diff: http://reviewboard.kde.org/r/1942/diff > > > Testing > ------- > > This patch in a slightly different implementation (some code moved from > PowerDevil to Solid) > is being shipped with Kubuntu Karmic and Lucid and I think Fedora. I haven't > seen any bug reports > concering the OSD or incorrect brightness changes. > > Apart from that I tested the changes on my laptop and have verified that the > code correctly > respects the HAL property. > > > Thanks, > > Felix > > _______________________________________________ Plasma-devel mailing list [email protected] https://mail.kde.org/mailman/listinfo/plasma-devel
