[Kde-games-devel] Review Request: Konquest: add support for svg-themeable highlights for planet names and ships count

[Dmitry Suzdalev]

> On 2009-11-10 00:36:12, Parker Coates wrote:
> > trunk/KDE/kdegames/konquest/mapitems.cc, line 156
> > <http://reviewboard.kde.org/r/2086/diff/2/?file=14223#file14223line156>
> >
> >     If I'm not mistaken, creating a QPixmap with a width and height allocates memory for the pixel data even if you don't initialise that pixel data with anything. Since this pixmap is just "thrown away" if the key is already in the cache, it would probably make more sense to create a null QPixmap here and then only set the width and height if it wasn't found in the cache.

Nicely spotted, you're right :)


On 2009-11-10 00:36:12, Dmitry Suzdalev wrote:
> > Other than that, the patch looks good. Of course, I'm not familiar with the Konquest so I can't really comment on whether it makes sense for Konquest or not. :)

Well here i'm almost in the same situation, I was just implementing Eugenes request :) But at least I don't see that it's something bad. This is simply an extension that enables artists to do themes with certain color palette which otherwise would make playing impossible because names/numbers would not be visible.


- Dmitry


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


On 2009-11-09 12:53:48, Dmitry Suzdalev wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://reviewboard.kde.org/r/2086/
> -----------------------------------------------------------
> 
> (Updated 2009-11-09 12:53:48)
> 
> 
> Review request for KDE Games.
> 
> 
> Summary
> -------
> 
> Implemented an Eugene's (it-s) request:
> 
> For proper themeing he needs the way to "highlight" the planet names and ships count texts that are drawn near each planet.
> This is needed because otherwise they are always drawn in black text and this doesn't correlate well with some backgrounds.
> We communicated that it would be great for our artists to have two additional svg elements which will be drawn under the texts, 
> so they can presizely control highlights.
> 
> So that's what i've done - i've added a support for two svg elements with IDs "planet_name_bkgnd" and "planet_ship_count_bkgnd" and made 
> them to be rendered under the respective texts.
> 
> Along with this change i did some minor things:
> - Stored some often used values in variables - instead of calling functions and evaluating same expressions every time
> - Fixed a typo which might prevented caching to work properly - QString+int won't work :) it has to be QString+QString::number(int).
> - Replaced painting custom pixmap and setting it as alfamask with simpler pix.fill(Qt::transparent)
> - Do not use hardcoded font heights/widths - calculate them with QFontMetrics
> 
> 
> Diffs
> -----
> 
>   trunk/KDE/kdegames/konquest/mapitems.h 1045562 
>   trunk/KDE/kdegames/konquest/mapitems.cc 1045668 
> 
> Diff: http://reviewboard.kde.org/r/2086/diff
> 
> 
> Testing
> -------
> 
> Tested rendering. Looks ok.
> 
> 
> Thanks,
> 
> Dmitry
> 
>