[Marble-devel] Review Request 124511: Merged the decoration(124154) and the label placement(124498) review requests.

[Dennis Nienhüser]

> On Aug. 13, 2015, 7:10 p.m., Dennis Nienhüser wrote:
> > src/lib/marble/geodata/graphicsitem/GeoLineStringGraphicsItem.cpp, line 37
> > <https://git.reviewboard.kde.org/r/124511/diff/4/?file=394335#file394335line37>
> >
> >     All these properties might change. As a decoration is some sort of a child item that is created and destroyed by the containing GeoGraphicsItem, it should maintain its properties in sync with the containing item Therefore I'd only create the item here and then have GeoGraphicsItem take care of maintaining them (see comment there). If we remove the special handling of the z-value (see comment in GeometryLayer.cpp), this can be done generically in GeoGraphicsItem.

I'll drop the comments about changing the architecture. We can look into it later.


- Dennis


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


On Aug. 13, 2015, 12:19 a.m., Dávid Kolozsvári wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/124511/
> -----------------------------------------------------------
> 
> (Updated Aug. 13, 2015, 12:19 a.m.)
> 
> 
> Review request for Marble.
> 
> 
> Repository: marble
> 
> 
> Description
> -------
> 
> I changed a little bit the decoration creating method, it now uses a QList to store the decorations, so multiple decorations can be added this way. It was an idea for the street labeling, but it makes sense without that too.
> 
> 
> Diffs
> -----
> 
>   src/lib/marble/GeoPainter.h 7a757b9 
>   src/lib/marble/GeoPainter.cpp d04138c 
>   src/lib/marble/GeoPainter_p.h f0c4f9b 
>   src/lib/marble/MarbleGlobal.h cf2768f 
>   src/lib/marble/geodata/data/GeoDataFeature.h ea23cd8 
>   src/lib/marble/geodata/data/GeoDataFeature.cpp 6f330fb 
>   src/lib/marble/geodata/data/GeoDataFeature_p.h 496c356 
>   src/lib/marble/geodata/graphicsitem/GeoLineStringGraphicsItem.h 4842809 
>   src/lib/marble/geodata/graphicsitem/GeoLineStringGraphicsItem.cpp 4320c07 
>   src/lib/marble/geodata/graphicsitem/GeoPolygonGraphicsItem.h f469dfb 
>   src/lib/marble/geodata/graphicsitem/GeoPolygonGraphicsItem.cpp 81cfe9a 
>   src/lib/marble/graphicsview/GeoGraphicsItem.h 4ca4727 
>   src/lib/marble/graphicsview/GeoGraphicsItem.cpp b8fa693 
>   src/lib/marble/graphicsview/GeoGraphicsItem_p.h 01becfc 
>   src/lib/marble/layers/GeometryLayer.cpp 9eb3f50 
> 
> Diff: https://git.reviewboard.kde.org/r/124511/diff/
> 
> 
> Testing
> -------
> 
> It works on a freshly pulled version of Marble.
> 
> 
> Thanks,
> 
> Dávid Kolozsvári
> 
>

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