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

[Parker Coates]

> On 2009-10-08 17:54:05, Albert Astals Cid wrote:
> > Tried using callgrind or cachegrind to see if more or less instructions are called?

Argh! I did it again. Hit OK, but didn't Publish. Anyway, here's the reply I meant to leave 5 days ago.

I had considered that, but I don't really know how to get comparable data. If I had, say 30 worth of data, does more function calls mean better or worse performance? In some ways less could be better as it could be interpreted as more efficient. On the other hand, more function calls could be better as it might mean that the scene was repainted at a higher frame rate.

I'm interested in doing so profiling, but I don't have the *grind-foo to setup a sensible comparison or to properly interpret the results. If anyone has any pointers or links I'd appreciate it.


- Parker


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


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