----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://reviewboard.vidsolbach.de/r/264/#review257 -----------------------------------------------------------
Ship it! couple of minor things that could be improved, but the approach looks sane. woo! trunk/KDE/kdebase/workspace/plasma/applets/systemtray/protocols/fdo/x11embedcontainer.cpp <http://reviewboard.vidsolbach.de/r/264/#comment213> doesn't particularly matter here, but static const int's will give you better compile time reporting and end up generating the same code. trunk/KDE/kdebase/workspace/plasma/applets/systemtray/protocols/fdo/x11embedcontainer.cpp <http://reviewboard.vidsolbach.de/r/264/#comment210> for ++extra_safety that should probably be -MIN_TIME_BETWEEN_PAINTS - 1 trunk/KDE/kdebase/workspace/plasma/applets/systemtray/protocols/fdo/x11embedcontainer.cpp <http://reviewboard.vidsolbach.de/r/264/#comment212> you shouldn't need to delete the timer; prevents having to new it every time in the paintEvent as well. =) trunk/KDE/kdebase/workspace/plasma/applets/systemtray/protocols/fdo/x11embedcontainer.cpp <http://reviewboard.vidsolbach.de/r/264/#comment211> should be parented to something so it will get autodeleted on object destruction - Aaron On 2008-11-08 19:40:24, Jason Stubbs wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > http://reviewboard.vidsolbach.de/r/264/ > ----------------------------------------------------------- > > (Updated 2008-11-08 19:40:24) > > > Review request for Plasma. > > > Summary > ------- > > There are some situations - such as overlapping sibling widgets - that can > cause a painting loop that can make plasma severely unresponsive. While those > issues should be fixed as they arise, I think that it is better that the > system tray be a little more robust as well. > > This patch limits paints to 10 per second. This number is arbitrary and > essentially limits the number of frames per second for animations, but can't > be too high as updates are fairly expensive. > > Also, is this method of using tokens ok? Is there a better way? > > > Diffs > ----- > > > trunk/KDE/kdebase/workspace/plasma/applets/systemtray/protocols/fdo/x11embedcontainer.h > > trunk/KDE/kdebase/workspace/plasma/applets/systemtray/protocols/fdo/x11embedcontainer.cpp > > Diff: http://reviewboard.vidsolbach.de/r/264/diff > > > Testing > ------- > > Resizing the panel to around 10 pixels will trigger excessive repainting. > With the patch, the paints being prevented is clearly visible and plasma > remains responsive. Resizing it too small will cause the icons to disappear > altogether and will cause things to become unresponsive, but kDebug()s > revealed that this was not due to painting. > > > Thanks, > > Jason > > _______________________________________________ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel
