[Marble-devel] Review Request: Implement what the KML spec says on placemarks regarding MultiGeometry and icons

[Dennis Nienhüser]

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



src/lib/geodata/data/GeoDataPlacemark.cpp
<http://git.reviewboard.kde.org/r/102591/#comment5796>

    This looks ugly code-wise. coordinate(bool*) should be coordinate(bool*=0) and deal with the null pointer as "not interested in the result". Then the "bool throwaway" is not needed here.



src/lib/geodata/data/GeoDataPlacemark.cpp
<http://git.reviewboard.kde.org/r/102591/#comment5797>

    Please deal with iconAtCoordinates=0. If you don't want to do that, the api docs must forbid to pass 0, but dealing with it is better as it avoids having to writing things like "bool throwaway" as above.



src/lib/layers/PlacemarkLayout.h
<http://git.reviewboard.kde.org/r/102591/#comment5798>

    "ok" without reading the api docs sounds like "the result is ok, i.e. valid coordinates". This is not very intuitive.


- Dennis


On Sept. 12, 2011, 6:03 p.m., Guillaume Martres wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/102591/
> -----------------------------------------------------------
> 
> (Updated Sept. 12, 2011, 6:03 p.m.)
> 
> 
> Review request for Marble and Thibaut Gridel.
> 
> 
> Summary
> -------
> 
> Implement what the KML spec says on placemarks regarding MultiGeometry and icons
> 
> From https://code.google.com/apis/kml/documentation/kmlreference.html#placemark
> "To give the user something to click in the 3D viewer, you would need to
> create a MultiGeometry object that contains both a Point and the other Geometry
> object."
> 
> 
> Diffs
> -----
> 
>   src/lib/geodata/data/GeoDataPlacemark.h 46842d7 
>   src/lib/geodata/data/GeoDataPlacemark.cpp 14634ed 
>   src/lib/layers/PlacemarkLayout.h bc2ceda 
>   src/lib/layers/PlacemarkLayout.cpp 34d7b7f 
> 
> Diff: http://git.reviewboard.kde.org/r/102591/diff
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Guillaume
> 
>

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