[Marble-devel] Review Request 111838: Change GeoDataIconStyle to use QPixmap instead of QImage internally

[Bernhard Beschow]

> On Aug. 1, 2013, 5:17 p.m., Torsten Rahn wrote:
> > Is this really a good idea? While I agree on the choice for view classes (like GeoGraphicsItems). I'm concerned about usage of QPixmap in GeoData* classes: They are models and could be generated in threads. However creation of QPixmaps in threads is a bad idea since they are created e.g. on the X-Server which only knows about the GUI thread. We already get warnings when using the graphics system X11 for this reason. For this reason I suggest to use QPixmap only in the view classes while preferring QImages on the model side.

For memory efficiency, it would be better to store pixmaps in the GeoData* classes and use them directly for rendering. For example, consider loading a pixmap in a single style instance once and use it for 1000+ GeoGraphicsItems.


- Bernhard


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


On Aug. 1, 2013, 4:25 p.m., René Küttner wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/111838/
> -----------------------------------------------------------
> 
> (Updated Aug. 1, 2013, 4:25 p.m.)
> 
> 
> Review request for Marble.
> 
> 
> Description
> -------
> 
> GeoDataIconStyle stores its icon as a QImage at the moment. Since these styles are meant for rendering and QImage is rather slow for this purpose, I changed it to use QPixmap internally.
> 
> 
> Diffs
> -----
> 
>   src/lib/MarbleWidgetPopupMenu.cpp ba27f36 
>   src/lib/VisiblePlacemark.cpp 42c1103 
>   src/lib/geodata/data/GeoDataFeature.h 4cd0374 
>   src/lib/geodata/data/GeoDataFeature.cpp 8a99a85 
>   src/lib/geodata/data/GeoDataIconStyle.h 5b4644f 
>   src/lib/geodata/data/GeoDataIconStyle.cpp d68d0bb 
> 
> Diff: http://git.reviewboard.kde.org/r/111838/diff/
> 
> 
> Testing
> -------
> 
> I applied the patch to master and did compile marble. I also compared the visual output of the icon style before and after the patch.
> 
> 
> Thanks,
> 
> René Küttner
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/marble-devel/attachments/20130801/beea82a9/attachment-0001.html>