[Kde-games-devel] Review Request: Port KBlocks to KGameRenderer

[Brian Croom]

> On 2010-08-23 08:50:45, Stefan Majewsky wrote:
> > /trunk/KDE/kdegames/kblocks/KBlocksScene.cpp, line 56
> > <http://reviewboard.kde.org/r/5081/diff/1/?file=34367#file34367line56>
> >
> >     Could you please check if this is still needed with revision 1167001 of libkdegames trunk?

Excellent! The latest code fixed the issue, although now I also had to fix another KBlocks destructor to stop it from calling delete on a bunch of pointers to its children which had already been deleted by KGameRenderer.


> On 2010-08-23 08:50:45, Stefan Majewsky wrote:
> > /trunk/KDE/kdegames/kblocks/KBlocksScene.cpp, lines 393-396
> > <http://reviewboard.kde.org/r/5081/diff/1/?file=34367#file34367line393>
> >
> >     Could you please do this with setBackgroundBrush, as in the other games, in order to avoid repeated spritePixmap() calls?

I'd love to, but I'm going to need some pointers on the right way to do this. In this case, the scene rect is often considerably smaller than the visualized part of the scene. In other words, the QRectF passed to the scene's drawBackground is different than the sceneRect. So when and where should the background be set? Is this still the responsibility of the scene, which would get the view size through QGraphicsScene::views()? Then I suppose it is necessary to connect a signal so it knows when the view size changes?


- Brian


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


On 2010-08-23 01:04:01, Brian Croom wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://reviewboard.kde.org/r/5081/
> -----------------------------------------------------------
> 
> (Updated 2010-08-23 01:04:01)
> 
> 
> Review request for KDE Games, Mauricio Piacentini and Stefan Majewsky.
> 
> 
> Summary
> -------
> 
> This straightforward patch makes the KBlocks rendering code use KGameRenderer instead of making its own KGameTheme and QSvgRenderer instances
> 
> 
> Diffs
> -----
> 
>   /trunk/KDE/kdegames/kblocks/KBlocksGraphics.h 1166816 
>   /trunk/KDE/kdegames/kblocks/KBlocksGraphics.cpp 1166816 
>   /trunk/KDE/kdegames/kblocks/KBlocksItemGroup.cpp 1166816 
>   /trunk/KDE/kdegames/kblocks/KBlocksScene.cpp 1166816 
>   /trunk/KDE/kdegames/kblocks/KBlocksSvgItem.h 1166816 
>   /trunk/KDE/kdegames/kblocks/KBlocksSvgItem.cpp 1166816 
>   /trunk/KDE/kdegames/kblocks/main.cpp 1166816 
> 
> Diff: http://reviewboard.kde.org/r/5081/diff
> 
> 
> Testing
> -------
> 
> I have tested playing games and switching themes, and encountered no problems.
> 
> 
> Thanks,
> 
> Brian
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://mail.kde.org/pipermail/kde-games-devel/attachments/20100823/867fb1a7/attachment.htm