[Kde-games-devel] Review Request: Simplify Card Painting in KPat

[Parker Coates]

> On 2009-10-08 09:45:29, Dmitry Suzdalev wrote:
> > Patch looks good (though I did a brief overview, not dived to deep). Mostly I wanted to tell you that there's an utility 'cpulimit' that you might find useful to emulate slow cpu. Check it in your distro packages, it should be there :)

I tried cpulimit. I had two copies of KPat (one pre patch, one post) playing the exact same game, maximised on side by side monitors, each throttled down to a measely 5% CPU and I still couldn't detect a difference. They were both choppy, but neither seemed consistently choppier than the other.

I'm not really sure if such a hacky comparison is enough grounds to throw out the optimisations.


- Parker


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://reviewboard.kde.org/r/1804/#review2587
-----------------------------------------------------------


On 2009-10-07 22:17:09, Parker Coates wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://reviewboard.kde.org/r/1804/
> -----------------------------------------------------------
> 
> (Updated 2009-10-07 22:17:09)
> 
> 
> Review request for KDE Games and Aaron Seigo.
> 
> 
> Summary
> -------
> 
> Lately I've been trying to clean up KPat codebase and have had a bit of success. Yesterday I had a read through the Card class and noticed that it contained lots of painting optimisations. 
> 
> - Each Card often checks to see if it is obscured by other cards.
> - Each Card has stores whether it is visible, hidden, or doesn't know.
> - Each Card keeps a list of cards it obscures and rechecks their visibility when it moves.
> - Only the exposed portions of a card are repainted.
> 
> I found this quite surprising as Card is a subclass of QGraphicsPixmapItem and I was under the impression that the GraphicsView framework handled a lot of these things internally. I did some testing an found that this code wasn't really working as expected. If 52 cards are pile directly on top of one another, dragging a card over top of them caused all 52 of them to repaint, even though only the top one is actually visible.
> 
> Of course KPat was one of the first KDE apps ported to Qt4 and GraphicsView. GraphicsView was brand new then and maybe these optimisations were needed to help it along. Since then things have changed quite a bit which may have internally broken these optimisations.
> 
> This patch radically simplifies the painting logic, but I'm not necessarily sure that this is a good idea. This is more of a request for comments. It's also a request for testing, especially if someone has a trunk environment set up on a lower powered machine and could compare the perceived performance before and after this patch.
> 
> 
> Diffs
> -----
> 
>   trunk/KDE/kdegames/kpat/card.h 1032448 
>   trunk/KDE/kdegames/kpat/card.cpp 1032448 
> 
> Diff: http://reviewboard.kde.org/r/1804/diff
> 
> 
> Testing
> -------
> 
> I see no regressions here, but I recently bought a new PC so I'm not sure I would notice a performance regression/improvement if there was one. When testing with QT_FLUSH_PAINT, I see roughly the same size and number of repaints both before and after the patch.
> 
> 
> Thanks,
> 
> Parker
> 
>