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

[Thomas Lübking]

> On April 26, 2014, 1:26 p.m., Thomas Lübking wrote:
> > wall.cpp, line 140
> > <https://git.reviewboard.kde.org/r/117785/diff/1/?file=268447#file268447line140>
> >
> >     here's 64px alignment to reduce reallocation
> 
> Roney Gomes wrote:
>     If I understood correctly, you are ensuring a 64 pixels margin between tiles, right? What I do not understand very well is the motivation behind this. Could you explain what do you mean when you say this approach is going to "reduce reallocation"?

> you are ensuring a 64 pixels margin between tiles
No.
The function so far is called with pixel growing boundingRect* (in either direction), ie. you're supposed to create 16x16, 16x17, 16x18, 16x19, ... pixmaps (and omit the old one)
Now, instead the pixmap allocates some more space so it can be re-used on further occasions.
It's less an issue for the raster graphicssystem (because memory allocation is smart enough for this), but on native/opengl this statement:

QPixmap pix(size);
pix.fill(Qt::transparent);

Needs to allocate two pixmaps/textures (and the latter one ARGB) - this is generally and was esp. terrible expensive on the prop. nvidia driver and i wanted to see whether this could improve Ians situation.


- Thomas


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


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/fcb5c042/attachment.html>