[Kde-games-devel] Review Request 117785: fix performance issues

[Roney Gomes]

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/117785/#review56672
-----------------------------------------------------------


Despite the issues highlighted below, your patch looks OK to me. Let's wait for Ian's next comments and see how things behave on OSX.


board.cpp
<https://git.reviewboard.kde.org/r/117785/#comment39518>

    Not extremely related to your patch. But people were talking about QGraphicsScene becoming obsolete in QT5.
    
    As this game is going to be ported soon, I'm afraid of future headaches. What do you know about it?



wall.cpp
<https://git.reviewboard.kde.org/r/117785/#comment39519>

    As this variable is now needed only in the first if statement then we can inline the condition it's representing and spare our memory a few bytes.



wall.cpp
<https://git.reviewboard.kde.org/r/117785/#comment39520>

    I still don't get what you intend with this variable.
    
    Is it signaling that the walls need to be repainted? If so, I believe we can provide it a better name.



wall.cpp
<https://git.reviewboard.kde.org/r/117785/#comment39522>

    This whitespace wasn't here before. ;-)


- Roney Gomes


On April 26, 2014, 1:17 p.m., Thomas Lübking wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/117785/
> -----------------------------------------------------------
> 
> (Updated April 26, 2014, 1:17 p.m.)
> 
> 
> Review request for KDE Games, Ian Wadham and Roney Gomes.
> 
> 
> Repository: kbounce
> 
> 
> Description
> -------
> 
> Roney, I hope your the correct addressee - Ian referecend some "Roney" in a private mail.
> The patch looks bigger than it is, i had to partially fix coding style in order to read through it, sorry.
> 
> The MAJOR issue with the code is that it causes recursions on the eventloop by altering graphicsitems from the paint() call ("setPixmap()"), so i scrathced that.
> It seems a cause was that a full repaint was forced by setting some invisible fullsize pixmap item, while the only requirement for this was actually when finishing a wall - so i replaced that with an explicit full update call on occasion.
> 
> Also all non-raster graphicssystems HATE QPixmap::fill(Qt::transparent) causing a 24bit -> 32bit change and it's esp. bad on the nvidia driver.
> So i reduced that (and allocation) by re-using the old (ARGB) pixmap on a 64x64 alignment.
> Ian, you might esp. check that impact on OSX.
> 
> Last I cached the sprites, seemed reducing cpu on permanent paint (but i didn't really measure)
> 
> I'll also mark & comment the actual changes in the diff.
> 
> 
> Diffs
> -----
> 
>   board.h 75f66d4 
>   board.cpp 46b923b 
>   gameobject.h 9fb5788 
>   wall.h c56efa1 
>   wall.cpp df487a0 
> 
> Diff: https://git.reviewboard.kde.org/r/117785/diff/
> 
> 
> Testing
> -------
> 
> 100% cpu -> 1-2% cpu and more fluid =)
> No visible regression spotted.
> 
> 
> Thanks,
> 
> Thomas Lübking
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-games-devel/attachments/20140427/cf7c52e2/attachment.html>