Review Request: Prevent excessive painting of non-ARGB fdo system tray icons

[Aaron Seigo]

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