[Marble-devel] Review Request 110403: Add support for reading and writing the KML Model tag

Sun May 12 09:49:46 UTC 2013

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

Thanks, looks good. Comments below mostly address constness issues.

src/lib/geodata/data/GeoDataLocation.cpp
<http://git.reviewboard.kde.org/r/110403/#comment24076>

    GeoDataCoordinates has altitude() and setAltitude(), please use that instead of m_altitude

src/lib/geodata/data/GeoDataModel.h
<http://git.reviewboard.kde.org/r/110403/#comment24079>

    This should be const correct, i.e. instead of this method add two:

    const GeoDataLink& link() const;
    GeoDataLink& link();

    The implementation can be the same.

src/lib/geodata/data/GeoDataModel.h
<http://git.reviewboard.kde.org/r/110403/#comment24080>

    Same as with link() above, split into two methods:

    const GeoDataCoordinates& coordinates() const;
    GeoDataCoordinates& coordinates();

src/lib/geodata/data/GeoDataModel.h
<http://git.reviewboard.kde.org/r/110403/#comment24081>

    Two methods here as well

src/lib/geodata/data/GeoDataModel.h
<http://git.reviewboard.kde.org/r/110403/#comment24082>

    Two methods here as well

src/lib/geodata/data/GeoDataModel.h
<http://git.reviewboard.kde.org/r/110403/#comment24083>

    Two methods here as well

src/lib/geodata/data/GeoDataModel.h
<http://git.reviewboard.kde.org/r/110403/#comment24084>

    const QString &

src/lib/geodata/data/GeoDataModel.h
<http://git.reviewboard.kde.org/r/110403/#comment24085>

    const QString &

src/lib/geodata/data/GeoDataModel.cpp
<http://git.reviewboard.kde.org/r/110403/#comment24075>

    Please remove the members which are redundant to orientation(), scale(), location().

src/lib/geodata/handlers/kml/KmlAltitudeTagHandler.cpp
<http://git.reviewboard.kde.org/r/110403/#comment24077>

    Please remove

src/lib/geodata/handlers/kml/KmlElementDictionary.h
<http://git.reviewboard.kde.org/r/110403/#comment24078>

    Please keep the same indentation as above (make sure not to use tabs)

tests/TestModel.cpp
<http://git.reviewboard.kde.org/r/110403/#comment24086>

    Please #include "TestUtils.h" and remove this method

tests/TestModel.cpp
<http://git.reviewboard.kde.org/r/110403/#comment24087>

    Can you use a non-default refresh mode in the .kml and test for that?

- Dennis Nienhüser

On May 12, 2013, 9:11 a.m., Sanjiban Bairagya wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/110403/
> -----------------------------------------------------------
> 
> (Updated May 12, 2013, 9:11 a.m.)
> 
> 
> Review request for Marble.
> 
> 
> Description
> -------
> 
> Previously there was no way to read/write the <Model> tag in the kml files, if any. I added a few GeoData classes and tag-handlers which makes it possible now.
> 
> 
> This addresses bug 318288.
>     http://bugs.kde.org/show_bug.cgi?id=318288
> 
> 
> Diffs
> -----
> 
>   src/lib/geodata/CMakeLists.txt 1644d87 
>   src/lib/geodata/data/GeoDataAlias.h PRE-CREATION 
>   src/lib/geodata/data/GeoDataAlias.cpp PRE-CREATION 
>   src/lib/geodata/data/GeoDataGeometry.h 19cdd29 
>   src/lib/geodata/data/GeoDataLocation.h PRE-CREATION 
>   src/lib/geodata/data/GeoDataLocation.cpp PRE-CREATION 
>   src/lib/geodata/data/GeoDataModel.h PRE-CREATION 
>   src/lib/geodata/data/GeoDataModel.cpp PRE-CREATION 
>   src/lib/geodata/data/GeoDataOrientation.h PRE-CREATION 
>   src/lib/geodata/data/GeoDataOrientation.cpp PRE-CREATION 
>   src/lib/geodata/data/GeoDataResourceMap.h PRE-CREATION 
>   src/lib/geodata/data/GeoDataResourceMap.cpp PRE-CREATION 
>   src/lib/geodata/data/GeoDataScale.h PRE-CREATION 
>   src/lib/geodata/data/GeoDataScale.cpp PRE-CREATION 
>   src/lib/geodata/handlers/kml/KmlAliasTagHandler.h PRE-CREATION 
>   src/lib/geodata/handlers/kml/KmlAliasTagHandler.cpp PRE-CREATION 
>   src/lib/geodata/handlers/kml/KmlAltitudeModeTagHandler.cpp 827293b 
>   src/lib/geodata/handlers/kml/KmlAltitudeTagHandler.cpp 6884dba 
>   src/lib/geodata/handlers/kml/KmlCoordinatesTagHandler.cpp 1d479fe 
>   src/lib/geodata/handlers/kml/KmlElementDictionary.h 11d9f1d 
>   src/lib/geodata/handlers/kml/KmlElementDictionary.cpp 36a1b0e 
>   src/lib/geodata/handlers/kml/KmlHeadingTagHandler.cpp a0c4801 
>   src/lib/geodata/handlers/kml/KmlHrefTagHandler.cpp 052d5b0 
>   src/lib/geodata/handlers/kml/KmlLatitudeTagHandler.cpp 8040b97 
>   src/lib/geodata/handlers/kml/KmlLinkTagHandler.cpp a23acda 
>   src/lib/geodata/handlers/kml/KmlLocationTagHandler.h PRE-CREATION 
>   src/lib/geodata/handlers/kml/KmlLocationTagHandler.cpp PRE-CREATION 
>   src/lib/geodata/handlers/kml/KmlLongitudeTagHandler.cpp 88f534b 
>   src/lib/geodata/handlers/kml/KmlModelTagHandler.h PRE-CREATION 
>   src/lib/geodata/handlers/kml/KmlModelTagHandler.cpp PRE-CREATION 
>   src/lib/geodata/handlers/kml/KmlOrientationTagHandler.h PRE-CREATION 
>   src/lib/geodata/handlers/kml/KmlOrientationTagHandler.cpp PRE-CREATION 
>   src/lib/geodata/handlers/kml/KmlResourceMapTagHandler.h PRE-CREATION 
>   src/lib/geodata/handlers/kml/KmlResourceMapTagHandler.cpp PRE-CREATION 
>   src/lib/geodata/handlers/kml/KmlRollTagHandler.cpp 22404bf 
>   src/lib/geodata/handlers/kml/KmlScaleTagHandler.h PRE-CREATION 
>   src/lib/geodata/handlers/kml/KmlScaleTagHandler.cpp PRE-CREATION 
>   src/lib/geodata/handlers/kml/KmlSourceHrefTagHandler.h PRE-CREATION 
>   src/lib/geodata/handlers/kml/KmlSourceHrefTagHandler.cpp PRE-CREATION 
>   src/lib/geodata/handlers/kml/KmlTargetHrefTagHandler.h PRE-CREATION 
>   src/lib/geodata/handlers/kml/KmlTargetHrefTagHandler.cpp PRE-CREATION 
>   src/lib/geodata/handlers/kml/KmlTiltTagHandler.cpp 806593c 
>   src/lib/geodata/handlers/kml/KmlXTagHandler.h PRE-CREATION 
>   src/lib/geodata/handlers/kml/KmlXTagHandler.cpp PRE-CREATION 
>   src/lib/geodata/handlers/kml/KmlYTagHandler.h PRE-CREATION 
>   src/lib/geodata/handlers/kml/KmlYTagHandler.cpp PRE-CREATION 
>   src/lib/geodata/handlers/kml/KmlZTagHandler.h PRE-CREATION 
>   src/lib/geodata/handlers/kml/KmlZTagHandler.cpp PRE-CREATION 
>   src/lib/geodata/parser/GeoDataTypes.h 1bc7828 
>   src/lib/geodata/parser/GeoDataTypes.cpp ec7c414 
>   tests/CMakeLists.txt d4cfec5 
>   tests/TestModel.cpp PRE-CREATION 
> 
> Diff: http://git.reviewboard.kde.org/r/110403/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Sanjiban Bairagya
> 
>

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