[Marble-devel] Review Request 124154: Implemented decorations for GeoGraphicsItem.

[Dennis Nienhüser]

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


Looks lovely. I also tested it on my system and changed the animation rendering quality to high to see the view-dependent shadows change while dragging. Awesome :)


src/lib/marble/geodata/data/GeoDataFeature.h (line 85)
<https://git.reviewboard.kde.org/r/124154/#comment56063>

    If it doesn't take too much time, please split off the changes in this patch which clean up whitespace and commit them directly to master. That way the real changes are easier to spot (here and in the future when looking at the git history).



src/lib/marble/geodata/graphicsitem/GeoPolygonGraphicsItem.cpp (line 179)
<https://git.reviewboard.kde.org/r/124154/#comment56062>

    Looks great most times. For large viewports and small buildings the offset seems a bit high to me though, maybe tweak it a bit for that case?
    
    One way could be to take the screen size of the polygon into account.



src/lib/marble/graphicsview/GeoGraphicsItem.cpp (line 162)
<https://git.reviewboard.kde.org/r/124154/#comment56061>

    Is there a situation where m_isDecoration is not the same as (m_cachedDecoration != nullptr)? Otherwise I'd favor returning that here and spare the m_isDecoration member (to avoid redundancy).


- Dennis Nienhüser


On June 23, 2015, 12:01 p.m., Dávid Kolozsvári wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/124154/
> -----------------------------------------------------------
> 
> (Updated June 23, 2015, 12:01 p.m.)
> 
> 
> Review request for Marble.
> 
> 
> Repository: marble
> 
> 
> Description
> -------
> 
> This patch introduces a new virtual method `GeoGraphicsItem::createDecoration()` to enable the easy creation of decoration for every class thet is inherited from `GeoGraphicsItem`.
> 
> There are two new decorations implemented in this patch:
> 
> + Outlines for `GeoLineStringGraphicsItem`, for better street rendering in OSM vector tiles;
> + Fake3D effect for `GeoPolygonGraphicsItem` if they represent buildings in OSM vector tiles.
> 
> For performance reasons these decorations are only painted when the map quality is set to noraml or higher.
> 
> Other changes:
> 
> + Changed some of the color styles of OSM related items, e.g. streets, to match the default color scheme of [openstreetmap maps](http://www.openstreetmap.org/);
> + Implemented two new OSM feature: `HighwayFootway` and `HighwayCycleway`.
> 
> 
> Diffs
> -----
> 
>   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/124154/diff/
> 
> 
> Testing
> -------
> 
> In the end, I've decided to cache the decorations for every item, and thus the performance is much better than in the [previous review request](https://git.reviewboard.kde.org/r/124074/).
> This patch works for me and the results are better than I expected.
> 
> 
> Thanks,
> 
> Dávid Kolozsvári
> 
>

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