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

[Thomas Lübking]

> On April 27, 2014, 2:38 p.m., Roney Gomes wrote:
> > 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.

There's a pending issue about caching the sprites - the cache must be cleared on theme changes
(just in case: Ian somehow dragged me into this - i had not installed any games nor played kbounce before ;-)


> On April 27, 2014, 2:38 p.m., Roney Gomes wrote:
> > board.cpp, line 25
> > <https://git.reviewboard.kde.org/r/117785/diff/1/?file=268444#file268444line25>
> >
> >     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?

I don't think so - it's in the Qt5::Widgets component, not deprecated and afair serves as backend for QML
Where did you pick that?


> On April 27, 2014, 2:38 p.m., Roney Gomes wrote:
> > wall.cpp, line 101
> > <https://git.reviewboard.kde.org/r/117785/diff/1/?file=268447#file268447line101>
> >
> >     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.

I added it as safeguard against useless ::update() calls - ideally it should not be required and ::update() only be called if necessary.


- Thomas


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


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/55b3a076/attachment-0001.html>