Review Request: Analog clock painting performance

[Alain Boyer]

> On 2008-11-30 09:01:20, Aaron Seigo wrote:
> > hm. that seems a little suspect to me, but if grabbing the cached rects is really that slow, then Theme::findInRectsCache is probably what needs to be optimized.
> > 
> > can you post the cachegrind output somewhere, along with the clock configuration you used?
> > 
> >

It's not that it is very slow, it's that it was called 13 times for each update. Quite a bit of overhead when you have the m_fancyHands enabled!

http://aboyer.dyndns.org/kde/callgrind.out.before
http://aboyer.dyndns.org/kde/callgrind.out.after

>From the comments below, I guess the moral of the story is not to assume that the elements are properly positioned in the svg file. These changes now become pretty useless. I'll try to reduce the amount of actual painting instead of caching the rects.


> On 2008-11-30 09:01:20, Aaron Seigo wrote:
> > /trunk/KDE/kdebase/workspace/plasma/applets/analog-clock/clock.cpp, line 317
> > <http://reviewboard.vidsolbach.de/r/290/diff/1/?file=1565#file1565line317>
> >
> >     are you sure about that?

Actually, this is the one change that I'm pretty sure about! :) It ensures that the second hand is properly aligned with the clock face markers.

The "seconds" variable is computed at each update. This makes it overshoot by 1 degree and then return to the precise position instead of undershooting by 1 degree.


- Alain


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://reviewboard.vidsolbach.de/r/290/#review275
-----------------------------------------------------------


On 2008-11-30 07:32:56, Alain Boyer wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://reviewboard.vidsolbach.de/r/290/
> -----------------------------------------------------------
> 
> (Updated 2008-11-30 07:32:56)
> 
> 
> Review request for Plasma.
> 
> 
> Summary
> -------
> 
> This patch attempts to speed up the analog clock painting performance. Essentially, it caches the QRects of the theme elements and streamlines some of the painting code.
> 
> The speedup is explained below, however I think we can do even better. For example, we could draw the hour and minute hands and shadows to a pixmap and then use that when only the second hand needs an update. I'm not sure if this is the way to go? Comments or suggestions?
> 
> 
> Diffs
> -----
> 
>   /trunk/KDE/kdebase/workspace/plasma/applets/analog-clock/clock.h
>   /trunk/KDE/kdebase/workspace/plasma/applets/analog-clock/clock.cpp
> 
> Diff: http://reviewboard.vidsolbach.de/r/290/diff
> 
> 
> Testing
> -------
> 
> Valgrind tells me that this patch introduces a 1.46 speedup to the drawHand() method. This contributes to a 1.40 speedup of paintInterface(). Not bad.
> 
> Another way of looking at it, paintInterface() used to spend 63% of its time actually painting (m_theme->paint()). Now, paintInterface() spends 98% of its time painting. Hence, painting is now the bottleneck.
> 
> 
> Thanks,
> 
> Alain
> 
>