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

[Parker Coates]

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

Ship it!


In the end, I was able to find no gains from these optimisations and they were causing problems with my port to the QtKinetic animation framework. That change does bring significant performance improvements, so I think the choice is an easy one. :)

I just applied this patch as well as the port to Q*Animation. Check out trunk for a speedy new KPat!

- Parker


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