> On 2008-11-08 12:07:21, Aaron Seigo wrote:
> > a much easier, straightforward and foolproof way of doing this is to have a
> > QTime and a QTimer, and at the start of the paintEvent method have sth like:
> >
> > if (time.elapsed() < MIN_REFRESH_TIME) {
> > timer->start(MIN_REFRESH_TIME - time.elapsed() + 10);
> > return;
> > }
> >
> > time.restart();
> >
> > timer would be connected to a method that calls update. the rest of the
> > paint event would remain the same, and this should compress paints to be no
> > more than once every 1000/MIN_REFRESH_TIME (so 1/10th of a second if the
> > min time is 100, 1/4th of a second at 250, etc..).
> >
> > with your patch it will do 10 paints in rapid succession and just stall for
> > the remainder of the second. this would probably break overly-agressively
> > animated icons.
> >
> > the use of single shot timers in your patch also provides no way of
> > resetting the time to wait.
The 10 paints in rapid succession was half intentional in order to make it
obvious that there's a painting problem somewhere else. I have redone the patch
in line what you've said above though and it does simplify it a fair bit.
> On 2008-11-08 12:07:21, Aaron Seigo wrote:
> > trunk/KDE/kdebase/workspace/plasma/applets/systemtray/protocols/fdo/x11embedcontainer.cpp,
> > line 191
> > <http://reviewboard.vidsolbach.de/r/264/diff/1/?file=1466#file1466line191>
> >
> > this causes a repaint in one second whether it needs it or not, as
> > restorePaintTokens called update()
Only if all tokens were depleted...
> On 2008-11-08 12:07:21, Aaron Seigo wrote:
> > trunk/KDE/kdebase/workspace/plasma/applets/systemtray/protocols/fdo/x11embedcontainer.cpp,
> > line 189
> > <http://reviewboard.vidsolbach.de/r/264/diff/1/?file=1466#file1466line189>
> >
> > updateBackgroundImage will still get called 10 times.
Nope, only once. A guard was already in place so I didn't need to add one.
- Jason
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://reviewboard.vidsolbach.de/r/264/#review255
-----------------------------------------------------------
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
